Closed Bug 1101873 Opened 10 years ago Closed 9 years ago

Auto hide Menubar flickers (contents jumps up and down) when open Tools menu by mouse clicking

Categories

(Core :: XUL, defect)

35 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox34 --- unaffected
firefox35 + wontfix
firefox36 + fixed
firefox37 + fixed

People

(Reporter: alice0775, Assigned: hsteen)

References

Details

(Keywords: regression)

Attachments

(1 file)

[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
Flags: needinfo?(hsteen)
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
Flags: needinfo?(enndeakin)
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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/efcdd91cef75
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
I filed bug 1116522 on the issue described in comment 4.
Too late to take this in 35, please nominate for Aurora (36) uplift.
Hallvord, could you fill the uplift request? Thanks
Flags: needinfo?(hsteen)
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
Flags: needinfo?(hsteen)
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.