1063703 - Add brand colors to built-in search plugins

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Search Activity, defect, P1)

Description

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

Attachment: prev_branding_overview1.png

From Bug 1051973, we talked about trying to have the underline/active mode indicator (when the keyboard is up) to be more dynamic. Specifically, using the user's current search engine's branding color as the active mode color. 

If, they're using a custom search engine (say Bugzilla), the underline would still be our orange (#FF9500)

(Focus just on the active keyboard up screens in the mock attached)

Comment 1

[:Margaret Leibovic]

I already included this in my patches for bug 1051973, but I just hard-coded purple for Yahoo, so I'm going to morph this bug to be about adding support for the brand color to all the search plugins.

antlam, what colors do you want for the built-in plugins?

Comment 2

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #1)
> I already included this in my patches for bug 1051973

I mean bug 1049600.

Comment 3

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

Attachment: Screen Shot 2014-09-16 at 10.00.52 AM.png

Google: #4285F4
Amazon: #FE9B00
Twitter: #55ACEE
Bing: #FFBA00
Wikipedia: #000000

Let's try these out

Comment 4

[:Margaret Leibovic]

mconnor, any thoughts on how I should add this to our open search plugins?

I'm thinking of adding a tag like:

<Color>#4285F4</Color>

Comment 5

[:Margaret Leibovic]

Attachment: Use dominant color algorithm to get search engine color

Instead of adding more things to the search plugins, which will result in a lot more work to update all our locales, I think we should try to just use our dominant color algorithm to take care of this for us. Additionally, defaulting to our "Firefox orange" looks a bit odd if the icon is a totally contrasting color (e.g. Google).

Testing locally, it seems to work fine. If we really want to optimize the color for some engines, we could add an optional <Color> tag like I suggested before, but this feel like a nicer default than the orange.

My one concern would be performance, so maybe I should test this on some crappy devices before landing. However, it's not causing any noticeable delay on my N4. We could also look into caching this value if we need it to be available more quickly.

Comment 6

[:Margaret Leibovic]

Attachment: Use dominant color algorithm to get search engine color

Oops, this version is better.

Comment 7

[:Margaret Leibovic]

One thing I've already noticed is that our algorithm currently throws away black and white, returning transparent if we can't find a good dominant color. We'll need to add a parameter to tweak this so that it will work as desired for Wikipedia.

Comment 8

[:Margaret Leibovic]

Comment on attachment 8493440 [details] [diff] [review]
Use dominant color algorithm to get search engine color

Clearing review until I fix the outstanding issues with the algorithm.

Comment 9

[:Margaret Leibovic]

Attachment: Use dominant color algorithm to get search engine color (v2)

So the problem with the previous patch was that the dominant color algorithm will return white for icons that consist completely of black/white(ish) pixels. There is an applyTheshold parameter we could use to not ignore these black/white pixels, but that's not really what we want to do here, since we'd still want to get a color if it exists (e.g. yellow for the Amazon.com logo).

Basically, instead of defaulting to white for these black/white icons (e.g. Wikipedia), I think we should default to black. This agrees with the colors that antlam specified in comment 3.

Comment 10

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

Comment on attachment 8493986 [details] [diff] [review]
Use dominant color algorithm to get search engine color (v2)


>+        // Update the focused background color.
>+        int color = BitmapUtils.getDominantColor(bitmap);

This can take a long time, especially for bitmaps bigger than ~64px. Just keep it in mind. You might want to send in a cropped bitmap and/or use a background thread/asynctask

Comment 11

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> Comment on attachment 8493986 [details] [diff] [review]
> Use dominant color algorithm to get search engine color (v2)
> 
> 
> >+        // Update the focused background color.
> >+        int color = BitmapUtils.getDominantColor(bitmap);
> 
> This can take a long time, especially for bitmaps bigger than ~64px. Just
> keep it in mind. You might want to send in a cropped bitmap and/or use a
> background thread/asynctask

At least for now all these images are 32px.

If we do decide to move this to the background, we'll probably want to do something smart like cache the value in a pref, since it would look bad to delay showing the colored underline.

Comment 12

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/4f5b01ea7e7c

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f5b01ea7e7c

Comment 14

[Flaviu Cos]

Verified as fixed in build 35.0a1 (2014-10-02);
Device: Nexus 7 (Android 4.4.4).