1402849 - Middle-clicking item in 'Recent Highlights' section of Library Panel leaves menu open

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P5)

Description

[custom.firefox.lady [:tawn]]

1. Click 'Library Button'
2. Middle-click an item in the 'Recent Highlights' section that appears at the bottom of the panel. (Occasionally this section does not appear at all, in which case the bug is not reproducible.)

Actual Result: Item is opened in a new tab but menu remains open.
Expected: Item opened in new tab and menu closes.

Note that left-clicking items automatically closes the menu, so menu will close after Ctrl-click. Middle-click requires specific code to close menu. Perhaps by modifying onLibraryHighlightClick to close menu if event.button == 1. Haven't tested, but seems like it should work.

Comment 1

[Mike de Boer [:mikedeboer]]

(In reply to custom.firefox.lady from comment #0)
> Perhaps by modifying onLibraryHighlightClick to close menu if event.button
> == 1. Haven't tested, but seems like it should work.

I think that'd be a perfectly fine way to fix this.

Comment 2

[Andrea Casaccia]

I attempted a fix for this by modifying as follows onLibraryHighlightClick():

> onLibraryHighlightClick(event) {
>   let button = event.target;
>   if (event.button > 1 || !button._highlight) {
>     return;
>   }
>+  if (event.button == 1) {
>+    // Bug 1402849, close library panel after mid mouse click by giving focus to the window
>+    window.focus();
>+  }
>   window.openUILink(button._highlight.url, event);
> },

Looks like it's working, however I'm not sure if giving focus to window is the proper way of closing the library panel.
This would be my first contribution so I'm not sure how to proceed. Can anyone assist me?

Comment 3

[Andrea Casaccia]

Ping?

Comment 4

[Mike de Boer [:mikedeboer]]

Hi asacasia, that is almost right - calling `window.focus()` is a bit of a roundabout way to hide the panel. You can change that to `this.hide()` instead, which is the `PanelUI.hide` function.
Do you already know how to submit a patch? If not, you can read https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch specifically, which should tell you all the things you need to know.

Looking forward to reviewing your patch!

Comment 5

[Andrea Casaccia]

Hey mikdober, unfortunately `this.hide()` doesn't work. I noticed that if I use `this.toggle()` instead of `this.hide()`, the menu panel (the one anchored to the hamburger icon) is being opened, instead of closing the library one. So I'm guessing `this.hide()` is closing the wrong panel, effectively doing nothing in this scenario.

I couldn't figure out how ot target the library panel instead... any ideas?

(P.S. I also realized that my `window.focus()` hack is not actually working.)

Comment 6

[Mike de Boer [:mikedeboer]]

Thanks for taking a look!

I think it would help if you used `CustomizableUI.hidePanelForNode(button);` instead. Can you try that?

Comment 7

[Andrea Casaccia]

That does the trick, thank you! I will look into how submitting the patch later today.

Comment 8

[Andrea Casaccia]

Attachment: Bug 1402849 - Close library panel on mid mouse click

Review commit: https://reviewboard.mozilla.org/r/194850/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194850/

Comment 9

[Mike de Boer [:mikedeboer]]

Comment on attachment 8923709 [details]
Bug 1402849 - Close library panel on mid mouse click

https://reviewboard.mozilla.org/r/194850/#review200470

Thanks! And many congrats on your very first contribution to Firefox!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38ae6313a318
Close library panel on mid mouse click r=mikedeboer

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/38ae6313a318

Comment 12

[custom.firefox.lady [:tawn]]

Confirmed fixed on build 20171101220120; thanks.

Comment 13

[roxleitan]

Cannot reproduce the issue on Windows 10 x64 using Nightly 58.0a1 (2017-09-25).'Recent Highlights' section is missing in my new profile.

Based on comment 12 I will mark this bug as Verified-fixed.