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)
Tracking
()
NEW
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.
Comment 1•13 years ago
|
||
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.
Reporter | ||
Comment 2•13 years ago
|
||
Painful issue, that's a result of this bug, is described in bug 741506, fwiw.
Reporter | ||
Comment 3•13 years ago
|
||
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.
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
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)
Updated•10 years ago
|
Flags: needinfo?(mstange)
Comment 6•10 years ago
|
||
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
Comment 7•10 years ago
|
||
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)
Comment 8•10 years ago
|
||
(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
Comment 9•10 years ago
|
||
(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.
Updated•10 years ago
|
Whiteboard: [reader-ui]
Updated•10 years ago
|
Whiteboard: [reader-ui]
Updated•10 years ago
|
Whiteboard: [readinglist-v2]
Updated•9 years ago
|
Priority: -- → P4
Whiteboard: [readinglist-v2] → [readinglist-v2], tpi:+
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•