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

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: Bug 1374104 - Remove chrome://browser/skin/search-history-icon.svg in favor of the context-fill using chrome://browser/skin/history.svg.

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

Comment 2

[Jonathan Watt [:jwatt]]

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

Comment 3

[Johann Hofmann [:johannh]]

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.

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Screen Shot 2017-06-20 at 09.42.14.png

Comment 5

[Stephen Horlander [:shorlander]]

The icon shape is fine. It should match the existing grey color.

Comment 6

[Johann Hofmann [:johannh]]

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!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7e2c76c9f6d2