Closed Bug 1398390 Opened 7 years ago Closed 7 years ago

Right click on Library/Download/Page Action buttons produces two menus instead of one menu

Categories

(Firefox :: Toolbars and Customization, defect)

57 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- verified

People

(Reporter: Mark12547, Assigned: zbraniecki)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170908100218

Steps to reproduce:

Using Firefox Nightly 57, create a brand new profile.
Launch Firefox Nightly 57 using that brand new profile.
Right-click on the Library or Download icon.


Actual results:

Firefox displays the context menus for both Right-click and Left-click.


Expected results:

The Right-click should present only customize-related context menu.

Note: Left-click works fine by producing context menu specific to Libraries or specific to Downloads.

More information:

The Downloads Icon was added to the area right of the Address Bar on Nightly 2017-09-06 (so as of 2017-09-06 both Libraries and Downloads icons were present).

The double-menus for right-click started appearing on 2017-09-07.

Regression to follow.
mozregression-gui was run.

The last few lines of the log (entire log file attached) were:

> 2017-09-08T16:28:10: DEBUG : Found commit message:
> Bug 1395871 - Open toolbar menus on mousedown, rather than oncommand.
> 
> MozReview-Commit-ID: A3P4QBBgcB8
> 
> 2017-09-08T16:28:10: INFO : The bisection is done.
> 2017-09-08T16:28:10: INFO : Stopped



The Bisections Informations window contained:

app_name: firefox
build_date: 2017-09-07 05:42:27.652000
build_file: C:\Users\Mark12547\.mozilla\mozregression\persist\93d2637ff309--mozilla-inbound--target.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/Jsey9JXdSyGVO1g-L6sVyA/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: 93d2637ff309bbae9c09ef0eb085771d40ff7c66
pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=75b91420fff7c1ded50cc8dd9396590bc0172588&tochange=93d2637ff309bbae9c09ef0eb085771d40ff7c66
repo_name: mozilla-inbound
repo_url: https://hg.mozilla.org/integration/mozilla-inbound
task_id: Jsey9JXdSyGVO1g-L6sVyA
That's a regression from bug 1395871.

I'm taking it. Thank you for reporting!
Assignee: nobody → gandalf
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Additional system information:

Operating system: Windows 7 Home Premium SP1 (64-bit)

Firefox: Firefox Nightly 57.0a1 (2017-09-08) (64-bit) Build ID 20170908100218

Problem reproduced in:
- my Nightly configuration (8 WebExtensions, 0 Legacy extensions, 1 Theme)
- a _new_ Nightly Profile
- mozregression-gui (which was creating a brand new profile in each iteration)
Blocks: 1395871
Component: Untriaged → Toolbars and Customization
Keywords: regression
Comment on attachment 8906181 [details]
Bug 1398390 - Only react to mousedown if it is a left-click.

https://reviewboard.mozilla.org/r/177938/#review182980

r=me with the triple equals removed and the library-in-the-hamburger-panel change (re)moved to its own bug.

::: browser/components/customizableui/CustomizableUI.jsm:4233
(Diff revision 1)
>      switch (aEvent.type) {
>        case "aftercustomization":
>          this._enable();
>          break;
>        case "mousedown":
> +        if (aEvent.button !== 0) {

Here and elsewhere in this patch please use "loose" equals/not-equals, which is in the style guide and prevailing style in the files.

::: browser/components/customizableui/content/panelUI.inc.xul:606
(Diff revision 1)
>          <toolbarseparator/>
>          <toolbarbutton id="appMenu-library-button"
>                         class="subviewbutton subviewbutton-iconic subviewbutton-nav"
>                         label="&places.library.title;"
>                         closemenu="none"
> -                       oncommand="PanelUI.showSubView('appMenu-libraryView', this)"/>
> +                       onmousedown="PanelUI.showSubView('appMenu-libraryView', this)"/>

This doesn't look related, and if we were doing it we should do the same for the other subviews (help, more, developer tools) and the subviews of the library panel and the 'more' subview, and the subviews of the history and bookmarks subviews.

But really, I'm not convinced this really makes sense anyway, given that we presumably want to activate the other items in this menu onmouseup, so you can mousedown on the hamburger button and mouseup on the item you want. So activating this on mousedown would cause more inconsistency instead of less. As a result, I think this hunk shouldn't be in this patch, and if you want to proceed with it, it should be a separate bug to deal with the associated can of worms. :-)
Attachment #8906181 - Flags: review?(gijskruitbosch+bugs) → review+
Summary: Regression: Right click on Library or Download icon produces TWO menus instead of one. → Right click on Library/Download/Page Action buttons produces two menus instead of one menu
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a8147d55b02
Only react to mousedown if it is a left-click. r=Gijs
Backed out for failing browser-chrome's browser_overflow_anchor.js at least on macOS and Windows:

https://hg.mozilla.org/integration/autoland/rev/555e50d013a2f2fa91a1ef4958e30ace8d348854

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1a8147d55b0264d4d68c8a8bb0a7fdeb9590e491&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=129870714&repo=autoland

23:36:19     INFO - TEST-START | browser/components/downloads/test/browser/browser_overflow_anchor.js
23:37:04     INFO - TEST-INFO | started process screencapture
23:37:04     INFO - TEST-INFO | screencapture: exit 0
23:37:04     INFO - Buffered messages logged at 23:36:19
23:37:04     INFO - Entering test bound test_overflow_anchor
23:37:04     INFO - TEST-PASS | browser/components/downloads/test/browser/browser_overflow_anchor.js | Downloads button should not be overflowed. - 
23:37:04     INFO - Buffered messages finished
23:37:04     INFO - TEST-UNEXPECTED-FAIL | browser/components/downloads/test/browser/browser_overflow_anchor.js | Test timed out -
Flags: needinfo?(gandalf)
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #10)
> Backed out for failing browser-chrome's browser_overflow_anchor.js at least
> on macOS and Windows:
> 
> https://hg.mozilla.org/integration/autoland/rev/
> 555e50d013a2f2fa91a1ef4958e30ace8d348854
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=1a8147d55b0264d4d68c8a8bb0a7fdeb9590e491&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=retry&filter-
> resultStatus=usercancel&filter-resultStatus=runnable
> Failure log:
> https://treeherder.mozilla.org/logviewer.html#?job_id=129870714&repo=autoland
> 
> 23:36:19     INFO - TEST-START |
> browser/components/downloads/test/browser/browser_overflow_anchor.js
> 23:37:04     INFO - TEST-INFO | started process screencapture
> 23:37:04     INFO - TEST-INFO | screencapture: exit 0
> 23:37:04     INFO - Buffered messages logged at 23:36:19
> 23:37:04     INFO - Entering test bound test_overflow_anchor
> 23:37:04     INFO - TEST-PASS |
> browser/components/downloads/test/browser/browser_overflow_anchor.js |
> Downloads button should not be overflowed. - 
> 23:37:04     INFO - Buffered messages finished
> 23:37:04     INFO - TEST-UNEXPECTED-FAIL |
> browser/components/downloads/test/browser/browser_overflow_anchor.js | Test
> timed out -

This is because https://dxr.mozilla.org/mozilla-central/rev/d53ba311ca2f0c3d81d4a5e88a7449c18ec5e4b6/browser/components/downloads/test/browser/browser_overflow_anchor.js#41 should use a MouseEvent constructor (rather than new Event()) and/or pass button: 0.
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48a86fd5097d
Only react to mousedown if it is a left-click. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/48a86fd5097d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 57.0a1 (2017-09-11), so I'm marking this bug as VERIFIED. Thank you very much! \o/
Status: RESOLVED → VERIFIED
QA Contact: Virtual
And thank you, those who fixed and help fix this bug!

It's fixed for me, too.
Flags: needinfo?(gandalf)
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.