Closed Bug 985958 Opened 6 years ago Closed 6 years ago

Bookmarks toolbar open bookmark folder background depends on hover state when it shouldn't

Categories

(Firefox :: Theme, defect)

29 Branch
x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- verified
firefox30 --- verified
firefox31 --- verified

People

(Reporter: rvjanc, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [Australis:P4])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140318013849

Steps to reproduce:

Go to folder on bookmark toolbar

1 - Hover
2 - Click & Hold
3 - Click with cursor in dropdown
4 - Click with cursor out of dropdown


Actual results:

1 - Hover - Light gray background
2 - Click & Hold - Dark gray background
3 - Click with cursor in dropdown  - Light gray background
4 - Click with cursor out of dropdown - Dark gray background

See attached file.

This occurs on FF29b, FF30a2 and FF31a1.  I "think" I first noticed this when FF29 was in the Aurora channel but not sure.


Expected results:

1 - Hover - Light gray background
2 - Click & Hold - Dark gray background
3 - Click with cursor in dropdown  - DARK GRAY BACKGROUND
4 - Click with cursor out of dropdown - Dark gray background
This doesn't seem like a terribly big deal to me, but yes, we should ideally fix it.
Status: UNCONFIRMED → NEW
Component: Untriaged → Theme
Ever confirmed: true
Whiteboard: [Australis:P4]
Summary: Bookmark folder on toolbar background states are wrong → Bookmarks toolbar open bookmark folder background depends on hover state when it shouldn't
Here is the regression range

3-13-14 NO BUG
3-14-14 BUG THERE
Keywords: regression
(In reply to :Gijs Kruitbosch from comment #1)
> This doesn't seem like a terribly big deal to me, but yes, we should ideally
> fix it.

I agree "it doesn't seem" like a big deal; however, on a Mac it looks like the bookmark folder background FLASHES. It is rather annoying.

Hopefully with the regression range and knowledge of the code architecture (which I don't have), it will require only a few minutes to identify the "guilty" CSS.

Cheers all
Here is a hack to fix the bug. I inserted this in userChrome.css

#PlacesToolbarItems > .bookmark-item[open="true"] {
background-color: #555555 !important;
}

Now if this is inserted/modified in the actual code; all should be good.

At least for now the bookmark folder background is not FLASHING.

Cheers again and have a good weekend.
Anyone care to furnish the pushlog for this regression?
I broke this in bug 982835, apparently. I don't really understand why, though. :-\
Assignee: nobody → gijskruitbosch+bugs
Blocks: 982835
Status: NEW → ASSIGNED
Oh, yes I do - the [open] selector gets overridden by the :hover one because of the extra :not(.subviewbutton). Sigh.
So a fix is on the way? 

I had found this range of code using the DOMi but had no idea what other things were involved.

Simple fix, yes?
So I don't understand why we have two identical selectors that apply 'hover' and 'active' styling to [open] things, but I just got rid of the one that gets overridden, and adjusted the one that shouldn't be overridden by the actual ':hover' selector so that it doesn't. This fixes things for me. Mike, am I sane (at least as far as this patch goes)?
Attachment #8398732 - Flags: review?(mconley)
Comment on attachment 8398732 [details] [diff] [review]
fix CSS rules to make :active style work,

Review of attachment 8398732 [details] [diff] [review]:
-----------------------------------------------------------------

Nutty. I'm tempted to go back through the rev log and see what happened here, but I'm gonna guess an unintended collision of old and new code.

This looks better. Let's go for it.
Attachment #8398732 - Flags: review?(mconley) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/822b3855dc4b
Whiteboard: [Australis:P4] → [Australis:P4][fixed-in-fx-team]
Comment on attachment 8398732 [details] [diff] [review]
fix CSS rules to make :active style work,

(I am not around for a week after today, so pre-emptively asking for approval)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis / bug 982835
User impact if declined: bookmark folders have strange 'open' styling
Testing completed (on m-c, etc.): on m-c, locally
Risk to taking this patch (and alternatives if risky): low. Can't really get worse than it is right now. :-\
String or IDL/UUID changes made by this patch: none
Attachment #8398732 - Flags: approval-mozilla-beta?
Attachment #8398732 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/822b3855dc4b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4][fixed-in-fx-team] → [Australis:P4]
Target Milestone: --- → Firefox 31
Attachment #8398732 - Flags: approval-mozilla-beta?
Attachment #8398732 - Flags: approval-mozilla-beta+
Attachment #8398732 - Flags: approval-mozilla-aurora?
Attachment #8398732 - Flags: approval-mozilla-aurora+
Sorry but not familiar with how this works. Nightly is definitely fixed; when will the fix land on 30Aurora and 29Beta?
Thanks. I'll watch for them to show up.
Reproduced with Nightly from 2014-03-20. 
Verified as fixed with latest Nightly (Build ID: 20140401030203), latest Aurora (Build ID: 20140402004007) and Firefox 29 beta 4 (Build ID: 20140331125246) on Mac OS X 10.9.2:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Firefox/31.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
You need to log in before you can comment on or make changes to this bug.