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

RESOLVED FIXED in Firefox 60

Status

()

defect
P1
normal
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: winson.wen1, Assigned: jaws)

Tracking

({regression})

60 Branch
Firefox 60
Points:
---
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox58 unaffected, firefox59 unaffected, firefox60 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
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

Updated

a year ago
Component: Untriaged → Tabbed Browser
(Reporter)

Comment 1

a year ago
Mozregression points to Bug 1431189 - Add google chrome toolbar button color properties: r=jaws
(Reporter)

Comment 2

a year ago
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.
Thanks for the report. Vivek, could you please look into this?
Blocks: 1431189
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(vivek3zero)
Priority: -- → P1
Keywords: regression

Updated

a year ago
Assignee: nobody → ntim.bugs

Updated

a year ago
Flags: needinfo?(vivek3zero)
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
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.
Attachment #8949718 - Flags: review?(dao+bmo)
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.
Assignee: ntim.bugs → jaws
Status: NEW → ASSIGNED
dev-doc-needed to remove docs and compat data relating to button_background
Keywords: dev-doc-needed

Updated

a year ago
Attachment #8949718 - Attachment is obsolete: true

Comment 9

a year ago
mozreview-review
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
Attachment #8949857 - Flags: review?(dao+bmo) → review+

Comment 10

a year ago
Pushed by dgottwald@mozilla.com:
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa386052255a
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
(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! ;-)
Keywords: dev-doc-needed
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.
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
QA Whiteboard: [bugday-20180307]
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.