1399642 - Search icon in awesomebar is not correct icon

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Kate Hudson :k88hudson]

It looks like the icon is the awesome bar is not yet updated, it should be "chrome://browser/skin/find.svg" but it is "chrome://global/skin/icons/autocomplete-search.svg".

Comment 1

[Kate Hudson :k88hudson]

After further investigation we noticed that chrome://global/skin/icons/autocomplete-search.svg seems to be built differently on different OSs – would the correct fix here be to update find.svg to support both or to update the file resolving to that path to be the right icon/colour?

I'm asking since we'd like to use the same icon on the search box in about:newtab / activity stream

Comment 2

[Ed Lee :Mardak]

Fixing this might just fix bug 1396893 as activity stream is already using find.svg, so if that file is updated / packaged in a way that doesn't need css flipping, we should end up with a consistent icon.

Comment 3

[Dão Gottwald [:dao]]

find.svg is meant for the customizable Find toolbar button. That doesn't seem like the right icon to use here.

Comment 4

[Ed Lee :Mardak]

aaron, what should the icon be in the address bar?

Comment 5

[Aaron Benson]

The icon should be the new photon search icon that's being used in the new tab in-content search. The search icon currently in the awesombar is old. See the new Design System icon list: http://design.firefox.com/icons/viewer/

Comment 7

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [::dao] from comment #3)
> find.svg is meant for the customizable Find toolbar button. That doesn't
> seem like the right icon to use here.

We use the same icon for Search and Find. We could add a new icon just for search, but it would be a duplicate.

Comment 8

[Dale Harvey (:daleharvey)]

Attachment: Bug 1399642 - Use photon search icon in awesomebar.

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

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

https://reviewboard.mozilla.org/r/183550/#review188728

Like I said find.svg isn't right here, not visually but semantically. Please rename this file to search-glass.svg.

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183550/diff/1-2/

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

https://reviewboard.mozilla.org/r/183550/#review188758

It looks like this patch doesn't add search-glass.svg (please use hg move for that).

Comment 12

[Ed Lee :Mardak]

Am I looking in the wrong place for if it's `hg move`d or not?

browser/themes/shared/icons/search-glass.svg
Was browser/themes/shared/icons/find.svg

https://reviewboard.mozilla.org/r/183550/diff/2/#6
browser/themes/shared/icons/find.svg
browser/themes/shared/icons/search-glass.svg (moved)

Comment 13

[Dale Harvey (:daleharvey)]

I used git mv and cinnabar, but yeh mozreview / hg seems to have detected it as a move https://reviewboard.mozilla.org/r/183550/diff/2/#6

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

https://reviewboard.mozilla.org/r/183550/#review188830

Ugh, reviewboard.

Can you please file a followup on updating the icon in the search bar too?

Thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ade76994700a
Use photon search icon in awesomebar. r=dao

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ade76994700a

Comment 17

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/fa863ebbef3f831badda8ebb978adf71a86ba5cc
chore(styles): Backport Bug 1399642 - Use photon search icon in awesomebar. r=dao (#3614)

Comment 18

[Johann Hofmann [:johannh]]

Screenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=70158e4e215d784d1391db5e517b18727f4b3683&newProject=mozilla-central&newRev=5563e7da39b265ed1ba7796ec058bdbcf6f792f6&filter=about

Comment 19

[B.J. Herbison]

The inconsistency isn't totally fixed. The icon for "Find in Preferences" in about:preferences is different from the three other search icons I can see.

(This was originally reported in bug 1396893 which was closed as a duplicate of this bug.)

Comment 20

[Dão Gottwald [:dao]]

(In reply to B.J. Herbison from comment #19)
> The inconsistency isn't totally fixed. The icon for "Find in Preferences" in
> about:preferences is different from the three other search icons I can see.
> 
> (This was originally reported in bug 1396893 which was closed as a duplicate
> of this bug.)

I reopened bug 1396893.

Dale, please request beta approval here asap.

Comment 21

[Dale Harvey (:daleharvey)]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

Approval Request Comment
[Feature/Bug causing the regression]: New Photon Visual work
[User impact if declined]: Inconsistent Icons
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Nope
[Why is the change risky/not risky?]: Mostly an asset change + rename with some minor CSS
[String changes made/needed]:

Comment 22

[Valentina Claudia Ona]

I verified this issue using Nightly 58.0a1 with Build ID 20171001220301 on Windows 10 x64, Windows 7 x32, Ubuntu 16.04, Mac OS X  10.12.
I will mark this as verified fixed.

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8912168 [details]
Bug 1399642 - Use photon search icon in awesomebar.

New icons, taking it. Should be in 57b5

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eb043b38b438