Closed Bug 1156917 Opened 5 years ago Closed 5 years ago

Get higher resolution favicons for search engines

Categories

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

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 42
Tracking Status
firefox41 --- verified
firefox42 --- verified
fennec 41+ ---

People

(Reporter: mcomella, Assigned: Margaret)

References

Details

Attachments

(6 files, 1 obsolete file)

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?
Flags: needinfo?(krudnitski)
I don't really understand what you mean by 'swiping the assets' - could you clarify?
Flags: needinfo?(krudnitski)
(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.
Flags: needinfo?(krudnitski)
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!
Flags: needinfo?(krudnitski) → needinfo?(jnagel)
Or, we just need higher resolution icons from our default providers in general. :)
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?
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.
Flags: needinfo?(mconnor)
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).
Flags: needinfo?(jnagel)
Attached image prev_searchassets.png
Awesome!

FWIW, here's the set that Robin and I have put together. Think using these would be OK?
^ 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
Flags: needinfo?(jnagel)
:bnicholson Let's try the white-bg set first. My gut tells me they may look better and not so chiclet-y. Maybe not.
Flags: needinfo?(bnicholson)
Attached file android_qsearch_icons.zip (obsolete) —
Mike - Android assets!
Flags: needinfo?(michael.l.comella)
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
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(margaret.leibovic)
Without using the resource system, the updated amazon icon still looks pixellated (on my N4). We'll have to figure something new out.
See Also: → 1157922
Threw up a PR in bug 1157922.
Flags: needinfo?(bnicholson)
(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.
Flags: needinfo?(margaret.leibovic)
(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.
(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.
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
Flags: needinfo?(jnagel)
Talked to Joanne via email, seems like we're good to go. 

Will attach new assets, NI-ing her for context :)
Flags: needinfo?(jnagel)
Thanks Anthony!  We have approval from Amazon, Bing, DDG, Google, and Yahoo.
Flags: needinfo?(jnagel)
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!
Flags: needinfo?(michael.l.comella)
Darrin, check these out if you want to use them in iOS as well :)
Flags: needinfo?(dhenein)
When I dump the base64 version of the XXHDPI amazon asset into the locale file, the search engine stops appearing in the BrowserSearch screen. :\
Assignee: nobody → michael.l.comella
Flags: needinfo?(michael.l.comella)
(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).
(Fennec breaks with data URI including %)
Flags: needinfo?(dhenein)
Assignee: michael.l.comella → margaret.leibovic
tracking-fennec: --- → 41+
Flags: needinfo?(mconnor)
Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle
Attachment #8627399 - Flags: review?(mark.finkle)
Attached image screenshot
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!
Attachment #8627399 - Flags: review?(mark.finkle) → review+
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?
Flags: needinfo?(francesco.lodolo)
(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.
Flags: needinfo?(francesco.lodolo)
https://hg.mozilla.org/mozilla-central/rev/8cc1612b1b45
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Target Milestone: Firefox 41 → Firefox 42
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
Attachment #8627399 - Flags: approval-mozilla-aurora?
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 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! :)
Attachment #8627399 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in build 41.0a2 2015-07-16;
Devices: 
- Motorola Razr (Android 4.4.4).
- Asus Transformer Pad (Android 4.2.1).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.