Closed Bug 1063703 Opened 5 years ago Closed 5 years ago
Add brand colors to built-in search plugins
218.48 KB, image/png
37.23 KB, image/png
4.09 KB, patch
|Details | Diff | Splinter Review|
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?
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.
Google: #4285F4 Amazon: #FE9B00 Twitter: #55ACEE Bing: #FFBA00 Wikipedia: #000000 Let's try these out
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>
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.
Oops, this version is better.
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 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.
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 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+
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).
You need to log in before you can comment on or make changes to this bug.