Get higher resolution favicons for search engines

VERIFIED FIXED in Firefox 41

Status

()

VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: Margaret)

Tracking

unspecified
Firefox 42
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 verified, firefox42 verified, fennec41+)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments, 1 obsolete attachment)

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?
(Assignee)

Comment 6

3 years ago
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)

Comment 7

3 years ago
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)
Created attachment 8596689 [details]
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)
Created attachment 8596698 [details]
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.
Flags: needinfo?(bnicholson)
Created attachment 8596713 [details]
android_qsearch_icons.zip

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)
Created attachment 8596819 [details]
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.
See Also: → bug 1157922
Threw up a PR in bug 1157922.
Flags: needinfo?(bnicholson)
(Assignee)

Comment 15

3 years ago
(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)
No longer blocks: 1137483
(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.

Comment 18

3 years ago
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)
Created attachment 8605371 [details]
quicksearch_icons2.zip
Attachment #8596713 - Attachment is obsolete: true

Comment 21

3 years ago
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)
(Assignee)

Comment 25

3 years ago
(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)

Updated

3 years ago
Assignee: michael.l.comella → margaret.leibovic
tracking-fennec: --- → 41+
(Assignee)

Updated

3 years ago
Flags: needinfo?(mconnor)
(Assignee)

Comment 27

3 years ago
Created attachment 8627399 [details]
MozReview Request: Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle

Bug 1156917 - Use higher resolution favicons for search engines. r=mfinkle
Attachment #8627399 - Flags: review?(mark.finkle)
(Assignee)

Comment 28

3 years ago
Created attachment 8627401 [details]
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+
(Assignee)

Comment 31

3 years ago
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)
Depends on: 1179109
https://hg.mozilla.org/mozilla-central/rev/8cc1612b1b45
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41

Updated

3 years ago
Target Milestone: Firefox 41 → Firefox 42
(Assignee)

Comment 34

3 years ago
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?

Comment 35

3 years ago
Verified as fixed in build 42.0a1 2015-07-02;
Devices: 
- Motorola Razr (Android 4.4.4).
- Asus Transformer Pad (Android 4.2.1).
status-firefox42: fixed → verified
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+

Comment 38

3 years ago
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
status-firefox41: fixed → verified
You need to log in before you can comment on or make changes to this bug.