Open Bug 733419 Opened 13 years ago Updated 2 years ago

Don't wait for menus to close before updating them

Categories

(Core :: Widget: Cocoa, defect, P4)

x86
macOS
defect

Tracking

()

People

(Reporter: asaf, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [readinglist-v2], tpi:+)

The cocoa menu implementation doesn't update menus until they're closed. To be precise, menus are only updated when they're opened, if at all. This is bad for two reasons: 1. We want to build some menus asynchronously. livemarks are the use case in question, but consider the Wireless networks menu on mac as a good example. For the livemarks case, users will have to close and reopen the menu until this is fixed. 2. Visual feedback for closed menu (The "hint" you see when hitting Cmd+C for example) is broken for disabled/removed items until the menu is reopened.
livemarks is just the first case we went through the async menu building, though they wouldn't be worth this. The problem is that in future we may hit far more cases, especially now that we are moving towards all async APIs, we may have to asynchronously builds all Places views.
Blocks: 741506
Painful issue, that's a result of this bug, is described in bug 741506, fwiw.
In short, even though the menuitems are not removed, the dom elements are removed from the tree. This results in "zombie" menu items which do nothing when activated.
Blocks: 1141353
The Mac menu code already has some mutation observers for inserted and removed content; do we know if they are here to handle this case? I'm wondering if this is a bug in the current implementation or something not implemented yet. Markus, do you have an estimate of how much effort it would take to fix this? Is there any chance it could be fixed soon? This problem is annoying for the readinglist work in bug 1141353, and if there's any chance the fix here could be easy, we would be happy to avoid having to add a synchronous cache to be able to build Mac menus synchronously.
Flags: needinfo?(mstange)
I'll need to read the code again in order to answer that question. I'll try to do it tomorrow but I can't promise. It's possible that the mutations are applied only while the menus are closed, and that OS X doesn't allow mutating open windows, but I'll have to double-check whether that's true.
Flags: needinfo?(mstange)
Flags: needinfo?(mstange)
Our code doesn't look like it updates dynamically, only on open or close. I've seen mac menus update while they are open though. (the network menu does this) A quick search reveals some discussion at http://stackoverflow.com/questions/2808016/how-does-apple-update-the-airport-menu-while-it-is-open-how-to-change-nsmenu-w
Hmm, that sounds like the main challenge is to have the code that does the modification actually run while the menu is open. So any async events that cause us to update the menu would need to be processed with the menu open (it sounds like currently we don't process any events during that time). We could just try to change our run loop source runloop mode to include the suggested run loop mode. Is there an existing menu that I can test with? Or do I need to write my own XUL+JS code?
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] from comment #7) > Is there an existing menu that I can test with? Or do I need to write my own > XUL+JS code? You can test with the readinglist bookmarks menu if you remove the ifdef around it: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-menubar.inc#449 Then the steps to have some content in the readinglist: 1. In about:config set browser.readinglist.enabled to true. 2. Browser to a page that's compatible with reader mode (eg. a nytimes article). 3. Notice the reader mode icon that appears at the right side of the location bar. Click it to enter reader mode. 4. There's a toolbar on the left side of the reader mode. Click the "+" button (it will turn to a "-" button to confirm the item has been added to your reading list). 5. Repeat steps 3 and 4 as many times as you wa
(In reply to Florian Quèze [:florian] [:flo] from comment #8) > 5. Repeat steps 3 and 4 as many times as you wa want to have items in the list. 6. open the Bookmarks -> Reading List menu.
Whiteboard: [reader-ui]
Whiteboard: [reader-ui]
Whiteboard: [readinglist-v2]
Priority: -- → P4
Whiteboard: [readinglist-v2] → [readinglist-v2], tpi:+
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.