Closed Bug 1302086 Opened 3 years ago Closed 3 years ago

Delayed highlight on hover in bookmarks toolbar when dark version of Developer Edition theme is enabled

Categories

(Firefox :: Theme, defect)

50 Branch
All
Windows
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 52
Tracking Status
firefox48 --- wontfix
firefox49 --- unaffected
firefox50 --- verified
firefox51 --- verified
firefox52 --- verified

People

(Reporter: JuliaC, Assigned: Gijs)

References

Details

(Keywords: regression)

Attachments

(1 file)

[Affected versions]:
- latest Aurora 50.0a2 (2016-09-12)

[Affected platforms]:
- Windows 10 X64
- Windows 8.1 x86 

[Steps to reproduce]:
1. Launch Firefox
2. Assure that Developer Edition theme is enabled (it should be, since it is the default one for Aurora builds)
3. Click the "Show your bookmarks" button from the toolbar in order to open the bookmarks menu
4. Go to "Bookmarks Toolbar" section and check the "View Bookmarks Toolbar" option in order to enable the Bookmarks Toolbar
5. Hover one of the bookmarks saved on the Bookmarks Toolbar   

[Expected result]:
- The chosen bookmark is properly highlighted when it is in hover state 

[Actual result]:
- The chosen bookmark is highlighted twice: 
 * first time, in hover state
 * the second time, right after the hover state
 

[Regression range]:
- Last good revision: 9aea5a70f2679e59ad80a66785215a10c7073928
- First bad revision: f4aa3c3af595bf44fcfbc8ec7b6190ed207090e2
- Pushlog:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=9aea5a70f2679e59ad80a66785215a10c7073928&tochange=f4aa3c3af595bf44fcfbc8ec7b6190ed207090e2

* Potential regressor: Bug 1289510

[Additional notes]:
- I only reproduced this issue with Developer Edition theme enabled
I'm assuming this also reproduces with 51?
Blocks: 1289510
Flags: needinfo?(iulia.cristescu)
(In reply to Iulia Cristescu, QA [:IuliaC] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > I'm assuming this also reproduces with 51?
> 
> No, this issue is reproducible only with Fx Aurora (see the screencast
> https://drive.google.com/file/d/0B0nYKG6PRiCcenZPd0tDNUpxMVE/
> view?usp=sharing).

Is this the case with the dark version of the devedition theme as well?
Flags: needinfo?(iulia.cristescu)
(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Iulia Cristescu, QA [:IuliaC] from comment #2)
> > (In reply to :Gijs Kruitbosch from comment #1)
> > > I'm assuming this also reproduces with 51?
> > 
> > No, this issue is reproducible only with Fx Aurora (see the screencast
> > https://drive.google.com/file/d/0B0nYKG6PRiCcenZPd0tDNUpxMVE/
> > view?usp=sharing).
> 
> Is this the case with the dark version of the devedition theme as well?

Yes, you are right :) My mistake. 
So, the issue affects 50 and 51 Fx builds, being applied to dark Developer Edition theme.
49.0 build3 (20160912134115) is not affected.
Flags: needinfo?(iulia.cristescu)
Summary: Delayed highlight on hover in bookmarks toolbar when Developer Edition theme is enabled → Delayed highlight on hover in bookmarks toolbar when dark version of Developer Edition theme is enabled
Can you take a look at this since it was Rahki's regression? (Or maybe Dao?)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
I can't make sense of "the styling of the bookmarks toolbar items was missing background-origin/clip: padding-box when not hovered." When not hovered, we don't set a background color nor a border, do we?

Also, why are you using !important for that?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Dão Gottwald [:dao] from comment #7)
> I can't make sense of "the styling of the bookmarks toolbar items was
> missing background-origin/clip: padding-box when not hovered." When not
> hovered, we don't set a background color nor a border, do we?

We do for :active/[open] state, so if you open a folder and then move the mouse off it (not hovered), the styling changes, which is unexpected. The simplest solution seemed to be what the rule below already did and set the style on the item irrespective of :hover state.

> Also, why are you using !important for that?

Well, initially, because the analogous code for #nav-bar .toolbarbutton-1 does so:

https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#767-768

Why do both styles need this? AFAICT, because the shorthand use of background: overrides the background-clip. Here's a jsbin which shows this:

http://jsbin.com/fakanababi/edit?html,css,output

adding !important changes the border color of the two divs (because the border either does or doesn't overlap with the background).
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8796218 - Flags: review?(dao+bmo) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79b1455bc2d2
fix devedition issues with toolbar colours, improve styling of bookmarks toolbar items, r=dao
https://hg.mozilla.org/mozilla-central/rev/79b1455bc2d2
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Comment on attachment 8796218 [details]
Bug 1302086 - fix devedition issues with toolbar colours, improve styling of bookmarks toolbar items,

Approval Request Comment
[Feature/regressing bug #]: bug 1289510
[User impact if declined]:
- the devedition styling for toolbar buttons is suboptimal. This is largely restricted to nightly/aurora, of course.
- the bookmark toolbar buttons "open" style (for folders, live bookmarks, etc.) on Windows has a peculiar border style. This will also affect beta/release (50).
[Describe test coverage new/current, TreeHerder]: nope, styling only
[Risks and why]: low-ish, styling only, mostly devedition-related so the only real change affecting beta/release (50) is actually very small.
[String/UUID change made/needed]: nope
Attachment #8796218 - Flags: approval-mozilla-beta?
Attachment #8796218 - Flags: approval-mozilla-aurora?
Comment on attachment 8796218 [details]
Bug 1302086 - fix devedition issues with toolbar colours, improve styling of bookmarks toolbar items,

CSS only, low risk, Aurora51+, Beta50+
Attachment #8796218 - Flags: approval-mozilla-beta?
Attachment #8796218 - Flags: approval-mozilla-beta+
Attachment #8796218 - Flags: approval-mozilla-aurora?
Attachment #8796218 - Flags: approval-mozilla-aurora+
This is hitting conflicts uplifting to beta, could we get a rebased patch if this needs to land there?
Flags: needinfo?(gijskruitbosch+bugs)
Let's make sure this hasn't caused any regressions.
Flags: qe-verify+
The fix looks quite good. The issue is verified fixed on 
- 52.0a1 (2016-11-02)
- 51.0a2 (2016-11-02)
- 50.0 build1 (20161101104304).
I will set the corresponding flags.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.