the xbl binding for a menupopup is attached only once the menu is opened, after popupshowing is dispatched

RESOLVED FIXED in mozilla1.9alpha3

Status

()

Core
Widget: Cocoa
P2
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: mano, Assigned: mano)

Tracking

Trunk
mozilla1.9alpha3
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

In native menus, the XBL binding for a menupopup is only attached once you open the menu, after popupshowing is dispatched. This is OK unless that binding has its own popupshwoing handler which for that reason is only called starting the second time the menu is opened.

This breaks the places bookmarks menu on mac. Its binding sets a <handler event="popupwhoing"> for creating the menu content. Thus, the bookmarks menu (the non-static part, to be clear), is empty the first time it's opened.
Could we manually bound XBL here, synchronously? As things stands, it's only bound when dispatching the popupshowing event to a JS event listener.
You can do that from JS easily -- just access the node that the XBL is attached to.  Or are you working from C++?  In that case you could manually do the XPConnect wrapping, I guess...
C++, native mac menus code.
Summary: the xbl binding for a menupopup is attached only once the menu is opned, after popupshowing is dispatched → the xbl binding for a menupopup is attached only once the menu is opened, after popupshowing is dispatched
Created attachment 250230 [details] [diff] [review]
like this?

This doesn't seem to make any difference.
asaf asked me if I saw this problem on windows xp.  I'm not seeing the issue he describes in his original comment, but I am seeing this:

with my trunk build (with --enable-places-bookmarks), the first time I click the personal toolbar overflow chevron I get a flash of a menupopup with all the bookmarks and then it quickly adjusts to be the correct (overflowed) items.

the second time I click within a session, I don't get the flash.

note, this doesn't happen all the time, but enough times that I noticed it.
Say hello to my little friend bug 331297
> Say hello to my little friend bug 331297

thanks phil.  that's exactly what I'm seeing.
Asaf, at the point when OnCreate runs, what is popupContent->GetCurrentDoc()?  And is that call happening after layout has started?  nsElementSH::PostCreate only attaches the binding if:

1)  The content is in a document.
2)  The document has a presshell (GetShellAt(0) returns something).

I wouldn't be suprised if one of those two conditions is not yet true in your code.
(In reply to comment #8)
> Asaf, at the point when OnCreate runs, what is popupContent->GetCurrentDoc()? 

Same as the owner document.

> And is that call happening after layout has started?

Of popupContent or the document? The former has no frame (display:none), as for the later, the method is called when you open the menu.

> only attaches the binding if:
> 
> (GetShellAt(0) returns something).

it does.

Please see comment 0, the element is bounded here, just way too late (when this method dispatches popupshoing to a JS listener).
Hmm.  Can you breakpoint in the method you're changing, then when you hit that breakpoint in nsElementSH::PostCreate and step through to see what the deal is with the XBL binding attachment there?
Hrm, no PostCreate or GetNewOrUsed on the stack when WrapNative is called.
Oh, I misunderstood the meaning of the fourth argument of WrapNative. We're failing here:

http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcwrappednativeinfo.cpp#234

because I'm passing an IID of a non-scriptable interface.
Created attachment 250392 [details] [diff] [review]
patch

Thanks, Boris.
Attachment #250230 - Attachment is obsolete: true
Attachment #250392 - Flags: superreview?(bzbarsky)
Attachment #250392 - Flags: review?(joshmoz)
Comment on attachment 250392 [details] [diff] [review]
patch

Hrm, this wouldn't work for  empty menus (before binding).
Attachment #250392 - Attachment is obsolete: true
Attachment #250392 - Flags: superreview?(bzbarsky)
Attachment #250392 - Flags: review?(joshmoz)
Created attachment 255614 [details] [diff] [review]
patch
Attachment #255614 - Flags: superreview?(bzbarsky)
Attachment #255614 - Flags: review?(joshmoz)
I won't be able to review this in a reasonable timeframe (probably not before mid-April at this point).  Please ask someone else?
Attachment #255614 - Flags: superreview?(bzbarsky)
Attachment #255614 - Flags: superreview?(jst)

Updated

11 years ago
Attachment #255614 - Flags: review?(joshmoz) → review+
Priority: -- → P2
Target Milestone: mozilla1.9alpha1 → mozilla1.9alpha3

Updated

11 years ago
Attachment #255614 - Flags: superreview?(jst) → superreview+
Blocks: 370099
mozilla/widget/src/cocoa/Makefile.in 1.62
mozilla/widget/src/cocoa/nsMenuX.h 1.17
mozilla/widget/src/cocoa/nsMenuX.mm 1.36
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.