Added webextension's toolbar button at the right-end goes into overflow menu in compact mode

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
11 months ago
9 months ago

People

(Reporter: atlanto, Assigned: bwinton)

Tracking

({reproducible})

57 Branch
Firefox 57
All
Windows
reproducible
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 verified)

Details

(Whiteboard: [reserve-photon-visual])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

11 months ago
STR:
1. Install any webextension having toolbar-button such as uBlock Origin or Zoom Page WE (I tried with it) or ...
2. Customize Density to Compact

AR:
The right-end button is captured into overflow menu.

ER:
All buttons are visible.

mozregression:
 2:21.77 INFO: Last good revision: 5322c03f4c8587fe526172d3f87160031faa6d75
 2:21.77 INFO: First bad revision: d0068f9051be88eb4c97e28ec0a4f101ca4ff147
 2:21.77 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5322c03f4c8587fe526172d3f87160031faa6d75&tochange=d0068f9051be88eb4c97e28ec0a4f101ca4ff147

I believe this was triggered by bug 1388457. There must be insufficiency.

Comment 1

11 months ago
Somebody posted a workaround for this to MozillaZine, http://forums.mozillazine.org/viewtopic.php?p=14760820&sid=f11031d66046ab313483a1558621fb5b#p14760820

"To have the uBlock/uMatrix icons displayed with the number of blocked requests, requires an inner-padding of 6 (compact or not). Less than 6 and the icons will be pushed to the overflow menu (compact or not)."

This issue also happens with Bitwarden. It will stay in the toolbar if there are no available passwords, but disappears to the Overflow Menu immediately after navigating to a site that has an available password, which adds a numbered flag to the icon.

Comment 2

11 months ago
This feels like there's an overflow event that's generated on the button rather than on the toolbar that then triggers the button being moved, but the code here: https://dxr.mozilla.org/mozilla-central/rev/5322c03f4c8587fe526172d3f87160031faa6d75/browser/components/customizableui/CustomizableUI.jsm#4265-4272 is already meant to deal with that type of issue.

A minimal testcase here (or, even better, checking if my suspicion is near the mark and/or how we would update that code so it doesn't get confused by the layers of XBL bindings...) would be helpful. It seems unlikely that we will want to update inner padding on these buttons, but even though I don't think this really is, at its heart, a theme bug, I'll ping Dão to see if I'm missing something.
Blocks: 1388457
Flags: needinfo?(dao+bmo)

Comment 3

11 months ago
This happens for me as well. Seems to be when a number of objects blocked notification is on the ublock origin button.

Comment 4

11 months ago
(In reply to timbugzilla from comment #3)
> This happens for me as well. Seems to be when a number of objects blocked
> notification is on the ublock origin button.

So I guess this negative margin makes the toolbar overflow:
http://searchfox.org/mozilla-central/rev/13148faaa91a1c823a7d68563d9995480e714979/toolkit/themes/linux/global/toolbarbutton.css#121
Status: UNCONFIRMED → NEW
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
Ever confirmed: true
Keywords: reproducible
OS: Unspecified → Windows
Hardware: Unspecified → All

Comment 5

11 months ago
Workaround: manually inserting a toolbar item (e.g. "Bookmarks")
between the addon icons you want in the toolbar, and the overflow menu,
prevents icons being shoved to the overflow menu.

Put differently:

<URL Bar> <uBlock Origin, which I want in the toolbar> **<Bookmarks icon>** <Overflow icon>

... will leave uBlock Origin in the toolbar, but

<URL Bar> <uBlock Origin, which I want in the toolbar> <Overflow icon>

... will result in uBlock being shoved in the Overflow icons. 

Note: coming here from https://www.reddit.com/r/firefox/comments/6vqm8l/addons_stuck_in_overflow_menu/

Updated

11 months ago
Flags: needinfo?(dao+bmo)
Priority: -- → P3
Whiteboard: [reserve-photon-visual]
(Assignee)

Comment 6

11 months ago
Gijs: There is an overflow event generated on the button, but that gets correctly ignored.  I added some console logging, and this is what it shows:

  CustomizableUI:Overflow!!! overflow { target: <box.toolbarbutton-animatable-box>, isTrusted: true, view: ChromeWindow → browser.xul, detail: 2, layerX: 0, layerY: 0, pageX: 0, pageY: 0, which: 0, rangeOffset: 0, SCROLL_PAGE_UP: -32768 }  CustomizableUI.jsm:4296

  CustomizableUI:Overflow!!! overflow { target: <hbox#nav-bar-customization-target.customization-target>, isTrusted: true, view: ChromeWindow → browser.xul, detail: 1, layerX: 0, layerY: 0, pageX: 0, pageY: 0, which: 0, rangeOffset: 0, SCROLL_PAGE_UP: -32768 }  CustomizableUI.jsm:4296

  CustomizableUI:Still Overflow!!!

(The first event, for the button, doesn't get past the return statement.  The second one, for the nav-bar, does.)
Comment hidden (mozreview-request)

Updated

11 months ago
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: P3 → P1

Comment 8

11 months ago
mozreview-review
Comment on attachment 8903266 [details]
Bug 1389713 - Push the badge a little closer to the button in compact mode to avoid overflow. , ui-r=abenson

https://reviewboard.mozilla.org/r/175058/#review180344

::: browser/themes/shared/toolbarbuttons.inc.css:96
(Diff revision 1)
>  
> +/* Add an extra pixel of padding on the end to contain the badge and
> + * prevent it from overflowing the toolbar and being pushed into the
> + * overflow menu. */
> +toolbar .toolbarbutton-1.badged-button {
> +  padding-inline-end: 3px;

Instead of this, would it make sense to adjust .toolbarbutton-badge's margin-inline-end in compact mode?
Attachment #8903266 - Flags: review?(dao+bmo)
(Assignee)

Comment 9

11 months ago
Sure.  Are we happy with the 1px extra overlap in compact mode?

Compact: https://cl.ly/1M0F0L273T2w
Normal: https://cl.ly/1p410Q1d3l1T

Comment 10

11 months ago
This looks good to me!
Comment hidden (mozreview-request)

Comment 12

11 months ago
mozreview-review
Comment on attachment 8903266 [details]
Bug 1389713 - Push the badge a little closer to the button in compact mode to avoid overflow. , ui-r=abenson

https://reviewboard.mozilla.org/r/175058/#review180506

::: browser/themes/shared/toolbarbuttons.inc.css:120
(Diff revision 2)
>  
>  toolbar .toolbarbutton-1 > .toolbarbutton-icon {
>    /* horizontal padding + actual icon width */
>    max-width: calc(2 * var(--toolbarbutton-inner-padding) + 16px);
>  }
>  

Can you please move the new rule to this spot?

::: browser/themes/shared/toolbarbuttons.inc.css:187
(Diff revision 2)
>    color: inherit;
>  }
>  
> +/* Remove a pixel of margin on the end so that the badge doesn't
> + * overflow the toolbar and push the button into the overflow menu. */
> + :root[uidensity=compact] .toolbarbutton-badge {

nit: remove space before :root

Thanks!
Attachment #8903266 - Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 15

11 months ago
Pushed by bwinton@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e625bc399af9
Push the badge a little closer to the button in compact mode to avoid overflow. r=dao, ui-r=abenson
https://hg.mozilla.org/mozilla-central/rev/e625bc399af9
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Updated

11 months ago
Iteration: --- → 57.3 - Sep 19

Updated

11 months ago
Duplicate of this bug: 1395523

Updated

10 months ago
Flags: qe-verify? → qe-verify+
QA Contact: ovidiu.boca
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20170926220106

This issue has been verified on latest Firefox Nightly Build ID: 20170926220106 on Windows 8.1 x64, Mac OS 10.12 and Ubuntu 14.04 and I cannot reproduce it. Now, webextensions that are added in the browser toolbar at the right-end and they don't go into overflow menu when switching to compact mode.

Updated

10 months ago
Status: RESOLVED → VERIFIED

Updated

10 months ago
status-firefox58: --- → verified

Updated

9 months ago
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.