Closed Bug 1063703 Opened 5 years ago Closed 5 years ago

Add brand colors to built-in search plugins

Categories

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

x86
Android
defect

Tracking

(firefox35 verified)

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified

People

(Reporter: antlam, Assigned: Margaret)

References

Details

Attachments

(3 files, 2 obsolete files)

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)
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?
Blocks: search
Flags: needinfo?(alam)
Summary: Using Search engine branding color as active mode indicator for underline → Add brand colors to built-in search plugins
(In reply to :Margaret Leibovic from comment #1)
> I already included this in my patches for bug 1051973

I mean bug 1049600.
Blocks: 1049600
No longer blocks: 1051973
Google: #4285F4
Amazon: #FE9B00
Twitter: #55ACEE
Bing: #FFBA00
Wikipedia: #000000

Let's try these out
Flags: needinfo?(alam)
Assignee: nobody → 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>
Flags: needinfo?(mconnor)
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.
Attachment #8493438 - Flags: review?(bnicholson)
Oops, this version is better.
Attachment #8493438 - Attachment is obsolete: true
Attachment #8493438 - Flags: review?(bnicholson)
Attachment #8493440 - Flags: review?(bnicholson)
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.
Priority: -- → P1
Flags: needinfo?(mconnor)
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.
Attachment #8493440 - Flags: review?(bnicholson)
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.
Attachment #8493440 - Attachment is obsolete: true
Attachment #8493986 - Flags: review?(bnicholson)
Attachment #8493986 - Flags: review?(mark.finkle)
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
Attachment #8493986 - Flags: review?(mark.finkle) → review+
(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.
Attachment #8493986 - Flags: review?(bnicholson) → review+
https://hg.mozilla.org/mozilla-central/rev/4f5b01ea7e7c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Verified as fixed in build 35.0a1 (2014-10-02);
Device: Nexus 7 (Android 4.4.4).
Status: RESOLVED → VERIFIED
Depends on: 1078301
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.