Stop using the :not(:target) selector in chrome://browser/skin/search-history-icon.svg

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

(Blocks 1 bug, {perf})

unspecified
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 2

2 years ago
I should note that Dao told me that search-history-icon.svg references should be
replaced with history.svg references.

Comment 3

2 years ago
mozreview-review
Comment on attachment 8878936 [details]
Bug 1374104 - Remove chrome://browser/skin/search-history-icon.svg in favor of the context-fill using chrome://browser/skin/history.svg.

https://reviewboard.mozilla.org/r/150174/#review155498

I think this icon was supposed to be different and I don't know if we can just change that. Let's ask shorlander. Stephen, I've attached a screenshot of the search history icon change in this patch. It's the same as the toolbar history icon now. If you compare it to the current version, should we keep the two icons separate or do you agree that we can only use the toolbar icon for both?

Also, as you can see, the icon is black instead of graytext for me on OSX. I'm not sure what's going on, the code looks correct to me. I haven't had any luck in inspecting these list items (and IRC hasn't been able to help so far), so we might have to find a different way to figure out why the context-fill is not applied here.
Attachment #8878936 - Flags: review?(jhofmann)
Flags: needinfo?(shorlander)
(Assignee)

Updated

2 years ago
Depends on: 1374614
The icon shape is fine. It should match the existing grey color.
Flags: needinfo?(shorlander)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8878936 [details]
Bug 1374104 - Remove chrome://browser/skin/search-history-icon.svg in favor of the context-fill using chrome://browser/skin/history.svg.

https://reviewboard.mozilla.org/r/150174/#review156158

Thanks, Stephen!

This works for me now, thank you!
Attachment #8878936 - Flags: review+

Comment 7

2 years ago
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e2c76c9f6d2
Remove chrome://browser/skin/search-history-icon.svg in favor of the context-fill using chrome://browser/skin/history.svg. r=johannh

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e2c76c9f6d2
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.