1436749 - Tab bar overflow button don't pulse when opening new tab

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

[Winson Wen]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180208103049

Steps to reproduce:

Open a page with links.
Open links in new tabs until the tab bar overflows.
Open more links in new tabs.


Actual results:

Tab bar overflow button (the < > buttons) didn't pulse/highlight.


Expected results:

Tab bar overflow button pulses/highlights to indicate a new tab was opened off screen

Comment 1

[Winson Wen]

Mozregression points to Bug 1431189 - Add google chrome toolbar button color properties: r=jaws

Comment 2

[Winson Wen]

Using Browser Toolbox to disable this rule fixes it

#PersonalToolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active),
.tabbrowser-arrowscrollbox > .scrollbutton-up:not([disabled=true]),
.tabbrowser-arrowscrollbox > .scrollbutton-down:not([disabled=true]),
.findbar-button:not(:-moz-any([checked="true"],[disabled="true"])) > .toolbarbutton-text,
toolbarbutton.bookmark-item:not(.subviewbutton):not([disabled="true"]):not([open]),
toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-icon,
toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-text,
toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-badge-stack {
  background-color: var(--lwt-toolbarbutton-background, transparent);
  color: inherit;
}

I'm guessing that .tabbrowser-arrowscrollbox > .scrollbutton-down:not([disabled=true]) from above is more specific than .tabbrowser-arrowscrollbox > .scrollbutton-down[highlight] so it screws up the highlight, but I don't know enough about css to fix this.

Comment 3

[Dão Gottwald [:dao]]

Thanks for the report. Vivek, could you please look into this?

Comment 4

[Tim Nguyen :ntim]

Attachment: Bug 1436749 - Fix selector specificity of toolbar button background rule.

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

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8949718 [details]
Bug 1436749 - Fix selector specificity of toolbar button background rule.

https://reviewboard.mozilla.org/r/219048/#review224792

::: browser/themes/shared/toolbarbuttons.inc.css:179
(Diff revision 1)
> -.findbar-button:not(:-moz-any([checked="true"],[disabled="true"])) > .toolbarbutton-text,
> -toolbarbutton.bookmark-item:not(.subviewbutton):not([disabled="true"]):not([open]),
> -toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-icon,
> -toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-text,
> -toolbar .toolbarbutton-1:not([disabled=true]):not([checked]):not([open]):not(:active) > .toolbarbutton-badge-stack {
> +.findbar-button > .toolbarbutton-text,
> +toolbarbutton.bookmark-item:not(.subviewbutton),
> +toolbar .toolbarbutton-1 > .toolbarbutton-icon,
> +toolbar .toolbarbutton-1 > .toolbarbutton-text,
> +toolbar .toolbarbutton-1 > .toolbarbutton-badge-stack {
>    background-color: var(--lwt-toolbarbutton-background, transparent);

Ahem, I thought we had agreed on not implementing button_background for the time being.

Our toolbar button's weren't designed for this, e.g. when all buttons have a background, they may seem too close together depending on the UI density setting. And I surely would prefer not having to worry about this rule going forward.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Sorry, this is my fault for accidentally including button_background in https://bugzilla.mozilla.org/show_bug.cgi?id=1347184#c5.

I'll prepare a backout of the button_background support.

Comment 7

[Tim Nguyen :ntim]

dev-doc-needed to remove docs and compat data relating to button_background

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1436749 - Backout support for button_background since it was not supposed to be supported.

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

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8949857 [details]
Bug 1436749 - Backout support for button_background since it was not supposed to be supported.

https://reviewboard.mozilla.org/r/219170/#review225008

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa386052255a
Backout support for button_background since it was not supposed to be supported. r=dao

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/fa386052255a

Comment 12

[Chris Mills [:cmills]]

(In reply to Tim Nguyen :ntim from comment #7)
> dev-doc-needed to remove docs and compat data relating to button_background

I don't think we've got any docs related to this, so removing ddn for now. Let me know if I'm wrong, and help me find the missing docs! ;-)

Comment 13

[Will Bamberg [:wbamberg]]

For future reference: it's certainly worth leaving dev-doc-needed on bugs like this, as otherwise I have no way of knowing that part of bug 1431189 was backed out.

Comment 14

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 60.0a1 (2018-02-08) on Windows 10, 64 Bit!
This bug's fix is verified with ltest Nightly!

Build ID   : 20180311220116
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0