Opened 6 years ago

Closed 6 years ago

#9823 closed bug (fixed)

BStringView's scripting is disabled

Reported by: ttcoder Owned by: axeld
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Sending "hey blah get Frame of View 0 of Window blah" fails if said view # 0 is a BStringView (by opposed to a BButton, BCheckBox ..etc which work very well).

Digging in the code, I see that the base BView::ResolveSpecifier() is overriden by a BStringView::ResolveSpecifier() which returns NULL without calling the base implementation at all (i.e. it disables BView scripting).

Attachments (2)

0001-Enable-BStringView-scripting.-Fixes-9823.patch (755 bytes) - added by jscipione 6 years ago.
Patch to call BView::ResolveSpecifier()
0001-BStringView-Enabled-scripting.-Fixes-9823.patch (3.6 KB) - added by jscipione 6 years ago.
Enables scripting allowing you to get and set Text and Alignment properties

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by ttcoder

I use a home-grown GUI debugging tool that retrieves the bounds of each view in a window, and just noticed it does not work for BStringViews. This is not a critical bug by any means, but it seems to be a simple overlook, so it would take just a few seconds to fix by removing the offending code here http://cgit.haiku-os.org/haiku/tree/src/kits/interface/StringView.cpp#n334

which clobbers/overrides/disables the scripting code here http://cgit.haiku-os.org/haiku/tree/src/kits/interface/View.cpp#n4208

Extra points if you guys also augment (not override/disable :-) the base code to add a "Label" property or some such, since this is the one thing that a BStringView has on top of a base BView, though not if it comes at the expense of removing the thingamadokie first, the one that disables the base "Frame", "Hidden" ..etc properties :-)

Changed 6 years ago by jscipione

Patch to call BView::ResolveSpecifier()

comment:2 Changed 6 years ago by jscipione

Has a Patch: set

comment:3 Changed 6 years ago by jscipione

I am not sure if this patch represents the correct course of action or not, somebody with more experience can chime in here. All it does is to call the ResolveSpecifier() method of the BView base class.

comment:4 Changed 6 years ago by ttcoder

If that's ok with Axel, Stippi et alia I say let's apply it.

That's probably better than my suggestion of removing the overriden method completely: keeping this "forwarding stub" as you do in this patch (with maybe a //todo: implement "Label" scriptable property as per #9823 comment thrown in or something) can serve as a reminder that someday one will have to implement the missing property, i.e. actually add flesh to ResolveSpecifier(), MessageReceived() ..etc to completely close this ticket. But the patch as-is will make my GUI debugging life easier already :-)

comment:5 Changed 6 years ago by korli

The patch is basically correct.

It would be desirable to compare with what name BeOS R5 defines for the StringView value property. Otherwise that would pretty much be what's used for BTextControl.

comment:6 Changed 6 years ago by phoudoin

Looking at TextControl.cpp, which handle "Value" itself, and base Control.cpp, which handle "Label", "Enabled" and a generic "Value" properties show how one could do that.

Sure, it should be kept BeOS R5 compatible, but it doesn't forbid to extend properties list to what make sense if it was lacking under R5, right?

Last edited 6 years ago by phoudoin (previous) (diff)

Changed 6 years ago by jscipione

Enables scripting allowing you to get and set Text and Alignment properties

comment:7 Changed 6 years ago by jscipione

Given that phoudoin though it would be okay to do more than strictly copy BeOS R5 here I've created an updated patch that in addition to enabling scripting support also allows the user to get and set the Text and Alignment properties of the object. I chose these properties because they are the fields archived by the view. If this is not warranted the first patch can be applied instead to only enable scripting without adding any additional scripting support.

comment:8 Changed 6 years ago by ttcoder

Finally booted my old BeOS 300 MHz batmobile this evening and... it looks like I was wrong to imply that adding properties (whilst re-enabling scripting) would be necessary to replicate BeOS R5 and completely tie-off this ticket -- see below: it's clear that Be had never got around to implementing BStringView properties. In fact there is no "suite/vnd.Be-stringview" at all, even though I know for a fact that the concerned target IS a BStringView.

So the Text And Alignment properties in jscipione's patch go the extra mile in fact, nice work!

Here's my scripting as generated by my debugging tool:

$ hey woogie getsuites of View 5 of View 0 of View 0 of View 1 of View 1 of View 0 of View 0 of Window 1
Reply BMessage(B_REPLY):
   "suites" (B_STRING_TYPE) : "suite/vnd.Be-view"
   "suites" (B_STRING_TYPE) : "suite/vnd.Be-handler"
   "messages" (B_PROPERTY_INFO_TYPE) :
        property   commands                            specifiers
--------------------------------------------------------------------------------
           Frame   B_GET_PROPERTY B_SET_PROPERTY       DIRECT
          Hidden   B_GET_PROPERTY B_SET_PROPERTY       DIRECT
            View   B_COUNT_PROPERTIES                  DIRECT
            View                                       INDEX REV.INDEX NAME

   "messages" (B_PROPERTY_INFO_TYPE) :
        property   commands                            specifiers
--------------------------------------------------------------------------------
          Suites   B_GET_PROPERTY                      DIRECT
       Messenger   B_GET_PROPERTY                      DIRECT
    InternalName   B_GET_PROPERTY                      DIRECT

   "error" (B_INT32_TYPE) : 0 (0x00000000)

$
$ hey woogie get Label of View 5 of View 0 of View 0 of View 1 of View 1 of View 0 of View 0 of Window 1
Didn't understand the specifier(s) (error 0x80002008)
$ hey woogie get Text of View 5 of View 0 of View 0 of View 1 of View 1 of View 0 of View 0 of Window 1
Didn't understand the specifier(s) (error 0x80002008)
$
      

comment:9 in reply to:  8 ; Changed 6 years ago by jscipione

Replying to ttcoder:

Finally booted my old BeOS 300 MHz batmobile this evening and... it looks like I was wrong to imply that adding properties (whilst re-enabling scripting) would be necessary to replicate BeOS R5 and completely tie-off this ticket... it's clear that Be had never got around to implementing BStringView properties.

I didn't look but I suspected that BStringView scripting wasn't available in BeOS because if they had added it there would have been some mention of it in the BeBook. That being said, it was probably an oversight on Be's part and not missing by design.

I am waiting for a developer who is more knowledgeable of Haiku's internals to chime in here and comment on if the second patch is acceptable or if it needs tweaking.

comment:10 in reply to:  9 Changed 6 years ago by leavengood

Replying to jscipione:

I am waiting for a developer who is more knowledgeable of Haiku's internals to chime in here and comment on if the second patch is acceptable or if it needs tweaking.

I may not be that developer since I'm not all that familiar with the scripting-related code...but nonetheless the last patch looks pretty good to me.

comment:11 Changed 6 years ago by jscipione

Resolution: fixed
Status: newclosed

Scripting support for BStringView added in hrev45807.

Note: See TracTickets for help on using tickets.