1156917 - Get higher resolution favicons for search engines

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The current assets are awkardly small in the work for bug 1137483 since we want to avoid pixellation.

Anthony suggested swiping the assets from the iPad build.

Kar, any partner issues with that?

Comment 1

[Karen Rudnitski [:kar]]

I don't really understand what you mean by 'swiping the assets' - could you clarify?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Karen Rudnitski [:kar] from comment #1)
> I don't really understand what you mean by 'swiping the assets' - could you
> clarify?

To use the assets (i.e. search engine favicons we ship) from the iPad version (which Anthony thinks are fairly large) in the Android search screen.

Comment 3

[Karen Rudnitski [:kar]]

Ah - I'm going to add Joanne, re-phrase the question (and you can correct me if I'm wrong), and she can tell you if she thinks there are issues.

You're asking whether we can use the search engine favicons that we are using on the iPad version and apply them to fennec because they will look better (better resolution)?

Joanne - do you see any issues from our search partners for using favicons that were created for iOS but applied to the Android platform? 

Thanks!

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Or, we just need higher resolution icons from our default providers in general. :)

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

This is quickly spiraling so let me sum up:

Both platforms (iOS and Android), could benefit from higher resolution icons for our current search providers that we support by default. I think we're just grabbing the favicons so they're pretty low res.

These include Yahoo, Google, Bing, Amazon, DuckDuck Go, Twitter, and Wikipedia.

Ideally, we could find the assets we need somewhere online and make sharper icons for our UI ourselves. They would also abide by each companies brand guidelines. That would be the fastest course of action forward.

Joanne/Kar - is this OK? Do we need to ask them to give us assets? or do we need to make sure that our these guys are OK with that before we move on it?

Comment 6

[:Margaret Leibovic]

In the past I've swapped out 16px icons for 32px icons, and I think we just took the icons from their web properties. But getting official icons would obviously be a preferred route :)

mconnor has also handled this in the past.

Comment 7

[Joanne Nagel]

I thought we did this exercise once before already when we were adding search to the new tab.  Before I request these can we check to be sure they haven't already been provided?

If not, can you be very specific about what sizes, resolution, etc. you want?  I can help with Google, Yahoo, Bing, Amazon, and DDG.  I do not have contacts for Twitter or Wikipedia, but they may have their branding guidelines online (at least Twitter used to).

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_searchassets.png

Awesome!

FWIW, here's the set that Robin and I have put together. Think using these would be OK?

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

^ DDG one needs to be cleaned up, not sure what went wrong there..

Joanne - on the Android side, they measure 32 dp squared. with 2 dp rounded corners. Maybe we can check with them if these (the ones we made) are OK? they should all abide to their brand guidelines

Comment 10

[Robin Andersen [:tecgirl]]

Attachment: search-provider-icons.zip

:bnicholson Let's try the white-bg set first. My gut tells me they may look better and not so chiclet-y. Maybe not.

Comment 11

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: android_qsearch_icons.zip

Mike - Android assets!

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Margaret, the assets are hard-coded into XML files at [1]. Preferably, we should lean on the Android resource system to provide larger assets when necessary.

I was thinking about overriding the iconURI in [2] when the name and/or identifier match and load the appropriate Android resource instead but I realize this does not help us for non en-US builds (and may do some swapping on non-en-US builds which means some assets will be pixellated and some will not be).

Should I just bump the asset sizes to 32px in the XML?

Also, how can we ensure the localized assets also get a size bump?

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/locales/en-US/searchplugins/
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/SearchEngine.java?rev=4e559dd64a13#33

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Android screenshot w/ new amazon asset, same system, N4

Without using the resource system, the updated amazon icon still looks pixellated (on my N4). We'll have to figure something new out.

Comment 14

[Brian Nicholson (:bnicholson)]

Threw up a PR in bug 1157922.

Comment 15

[:Margaret Leibovic]

(In reply to Michael Comella (:mcomella) from comment #12)
> Margaret, the assets are hard-coded into XML files at [1]. Preferably, we
> should lean on the Android resource system to provide larger assets when
> necessary.
> 
> I was thinking about overriding the iconURI in [2] when the name and/or
> identifier match and load the appropriate Android resource instead but I
> realize this does not help us for non en-US builds (and may do some swapping
> on non-en-US builds which means some assets will be pixellated and some will
> not be).

I do agree it sucks sending this image data over the bridge in a JSON blob, and it would be nice to include these as Android resources somehow, but you're right that we would have to deal with the l10n aspect. Maybe we can use some build fu to generate the correct resources from an l10n directory? I believe do something similar for suggested sites:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/locales/Makefile.in#117
http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/action/generate_suggestedsites.py

I think ckitching has looked into this in the past, it might be worth digging up what he's discovered.

Also, keep in mind, the search activity also has its own logic to parse these XML files, so we would need to make sure we don't break that.

> Should I just bump the asset sizes to 32px in the XML?

It's confusing, but the icons are already 32px. The XML says they're 16px because of this logic in the search service:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#2091

Times have changed since this was written, and the search service actually supports multiple icon sizes now, so maybe we can actually use the correct sizes in there now, but we may need to use a different API for getting out the right icon. It could be worth experimenting with just updating those width/height values and seeing if things still work.

I feel like we should see if we can support multiple icon sizes properly, since users can also install search plugins from the wild, and those may have multiple icon sizes.

> Also, how can we ensure the localized assets also get a size bump?

I think Pike or flod could help with that. We'll also want bigger icon sizes for the other search plugins that are shipped in different locales.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to :Margaret Leibovic from comment #15)
> I think Pike or flod could help with that. We'll also want bigger icon sizes
> for the other search plugins that are shipped in different locales.

I filed bug 1158405 to follow up with l10n.

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to :Margaret Leibovic from comment #15)
> I think ckitching has looked into this in the past, it might be worth
> digging up what he's discovered.

Looks like he was nudged towards using a scaling icon in FaviconView in bug 895423 comment 7 before he could do further research.

Comment 18

[Joanne Nagel]

Hi Anthony, sorry your request was buried in all the comments!  Can you email me the assets you want approved and I'll send to the partners?  If you just rounded the corners of their approved logo's then we should have an answer quickly.

Thanks, Joanne

Comment 19

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Talked to Joanne via email, seems like we're good to go. 

Will attach new assets, NI-ing her for context :)

Comment 20

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: quicksearch_icons2.zip

Comment 21

[Joanne Nagel]

Thanks Anthony!  We have approval from Amazon, Bing, DDG, Google, and Yahoo.

Comment 22

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Thanks Joanne! 

And we followed guidelines from Twitter and Wikipedia on their sites so that should be good too.

NI-ing mcomella for tracking the icon swaps!

Comment 23

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Darrin, check these out if you want to use them in iOS as well :)

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

When I dump the base64 version of the XXHDPI amazon asset into the locale file, the search engine stops appearing in the BrowserSearch screen. :\

Comment 25

[:Margaret Leibovic]

(In reply to Michael Comella (:mcomella) from comment #24)
> When I dump the base64 version of the XXHDPI amazon asset into the locale
> file, the search engine stops appearing in the BrowserSearch screen. :\

Is it encoded properly (or rather un-encoded)? I've had problems in the past where I accidentally tried to use a URL-encoded data URI (that doesn't work).

Comment 26

[Francesco Lodolo [:flod]]

(Fennec breaks with data URI including %)

Comment 27

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

Comment 28

[:Margaret Leibovic]

Attachment: screenshot

Comment 29

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8627399 [details]
MozReview Request: Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

https://reviewboard.mozilla.org/r/12225/#review10725

Ship It!

Comment 30

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8cc1612b1b45

Comment 31

[:Margaret Leibovic]

flod, is it okay for me to uplift these search plugin changes to aurora? And are you the right person to help update the icons in the localized search plugins as well?

Comment 32

[Francesco Lodolo [:flod]]

(In reply to :Margaret Leibovic from comment #31)
> flod, is it okay for me to uplift these search plugin changes to aurora?
Yes.

> And are you the right person to help update the icons in the localized search
> plugins as well?
Yes. I'll file a tracking bug, unfortunately it will take months, especially for searchplugins that are locale-specific.

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8cc1612b1b45

Comment 34

[:Margaret Leibovic]

Comment on attachment 8627399 [details]
MozReview Request: Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

Approval Request Comment
[Feature/regressing bug #]: bug 1137483 
[User impact if declined]: new search engine bar will have blurry icons
[Describe test coverage new/current, TreeHerder]: no automated test coverage, tested locally
[Risks and why]: low-risk, updating icons
[String/UUID change made/needed]: none

Comment 35

[Flaviu Cos]

Verified as fixed in build 42.0a1 2015-07-02;
Devices: 
- Motorola Razr (Android 4.4.4).
- Asus Transformer Pad (Android 4.2.1).

Comment 36

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8627399 [details]
MozReview Request: Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

Approved. It's nice to see that the fix has been verified too! :)

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5e0d14a59f61

Comment 38

[Flaviu Cos]

Verified as fixed in build 41.0a2 2015-07-16;
Devices: 
- Motorola Razr (Android 4.4.4).
- Asus Transformer Pad (Android 4.2.1).