Closed Bug 1101873 Opened 8 years ago Closed 8 years ago
Auto hide Menubar flickers (contents jumps up and down) when open Tools menu by mouse clicking
[Tracking Requested - why for this release]: [Tracking Requested - why for this release]: Problem is posted in http://forums.mozillazine.org/viewtopic.php?f=23&t=2890013 Steps To Reproduce: 1. Start browser with newly created profile (i.e, Titlebar is hidden, Menubar is autohide) 2. Press Alt key to unhide menubar 3. Click "Tools" menu 4. Repeat Step2 and 3 Actual Results: Auto hide Menubar flickers (contents jumps up and down) Expected Results: No flicker (no jump up and down) Regression window(m-i) Good: https://hg.mozilla.org/integration/mozilla-inbound/rev/37e8f50f6264 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20141011180624 Bad: https://hg.mozilla.org/integration/mozilla-inbound/rev/c9fa1a711dc7 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20141011184023 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=37e8f50f6264&tochange=c9fa1a711dc7 Suspect: 24fe4de37125 Hallvord R. M. Steen — Bug 923971 - Prevent global F10 shortcut from overriding devtools one. r=enndeakin
Neil, any idea what would cause this effect?
Flags: needinfo?(hsteen) → needinfo?(enndeakin)
Ops. Reviewing the patch I see it also changed mTarget->AddEventListener(NS_LITERAL_STRING("mousedown"), mMenuBarListener, true); to mTarget->AddSystemEventListener(NS_LITERAL_STRING("mousedown"), mMenuBarListener, true); (corresponding change for blur event and RemoveEventListener() calls). I'm building a new version without that change - hopefully the fix will still work.
Assignee: nobody → hsteen
Status: NEW → ASSIGNED
Just undoing some of the previous change..
Attachment #8529629 - Flags: review?(enndeakin)
Comment on attachment 8529629 [details] [diff] [review] bug-1101873-fix.patch What's happening here is that: 1. The nsMenuBarListener::MouseDown event listener is toggling the menubar inactive. 2. The mousedown listener for the menu (Tools) is responding, causing the menu to open and reactivate the menu. 3. The code in customizableui/content/toolbar.xml (and similarly in content/widgets/toolbar.xml) isn't handling this correctly. The _setInactiveAsync method called to set a timer. Sometimes the timer runs before the menubar is activated again in step 2 and sometimes it runs after. This cause some flicker. Part of the patch here is correct. The capturing listeners should not be using AddSystemEventListener, but the one non-capturing listener should be. It would be ideal to fix this properly but that's likely a more complex fix. Let's go with this for now to fix the bug, but file another bug on the underlying issue.
Attachment #8529629 - Flags: review?(enndeakin) → review+
Too late to take this in 35, please nominate for Aurora (36) uplift.
Hallvord, could you fill the uplift request? Thanks
Comment on attachment 8529629 [details] [diff] [review] bug-1101873-fix.patch Approval Request Comment [Feature/regressing bug #]: This bug is the regression report. Bug 1101873 [User impact if declined]: Annoying inconsistent menu behaviour [Describe test coverage new/current, TBPL]: Don't know [Risks and why]: very low, this is simply reversing a small part of a small change back to what it used to be [String/UUID change made/needed]: none
Attachment #8529629 - Flags: approval-mozilla-aurora?
Comment on attachment 8529629 [details] [diff] [review] bug-1101873-fix.patch thanks!
Attachment #8529629 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.