Closed Bug 765750 Opened 8 years ago Closed 7 years ago

Remove Yandex default search engine data from aboutHome.js

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 18

People

(Reporter: jaws, Assigned: fryn)

References

Details

(Whiteboard: [about-home])

Attachments

(1 file)

Bug 761592 made Google the default search engine worldwide, so we can now remove the code that specifically handles Yandex in /browser/base/content/abouthome/aboutHome.js.
Kev, do we still have Yandex as the default anywhere?
Right now we don't have any, http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=browser.search.defaultenginename&find=browser/chrome/browser-region says so.

I'd not want to loose the option to do that, though. Doesn't necessarily have to be yandex, too.
Attached patch patchSplinter Review
(In reply to Axel Hecht [:Pike] from comment #2)
> I'd not want to loose the option to do that, though. Doesn't necessarily
> have to be yandex, too.

The code is and will remain flexible. All that is needed if we get a new default search engine in some localization is to add its logo as a data URI (and parameters if applicable) to the list. We're simply removing Yandex from the list for now.
Assignee: nobody → fryn
Status: NEW → ASSIGNED
Attachment #667350 - Flags: review?(jaws)
Comment on attachment 667350 [details] [diff] [review]
patch

Let's get a review from Kev, too. It just occurs to me that yandex may or may not be using this for their partner builds.
Attachment #667350 - Flags: review?(kev)
Attachment #667350 - Flags: review?(jaws) → review+
Whiteboard: [about-home]
Why would we remove them? It'd be great if about:home actually followed what the user has selected as a default where possible (and if the default engine gets changed, then about home doesn't have any indicator of what the provider in use is if there's no logo on the next mstone update). I'd prefer we populate more of the popular search engines in the list, tbh (ideally, we'd add a param to open search plugins for branding and use that instead of what's hardcoded, but that's longer term).

Given Yandex's market penetration in ru, and that they're one of the search defaults (just not _the_ default), I'd rather we leave it in, and consider a general policy of adding brand files for all the general search engines in all locales (e.g. Yahoo!, Bing) with the option to add localized variants as well.
Comment on attachment 667350 [details] [diff] [review]
patch

Conditional r+ if we think this is the right thing to manage things. 

I'd prefer we keep it in, as Yandex Toolbar penetration is quite large, and it (I believe) updates the default engine pref, and having the Yandex logo in about:home to let users know where Search results will go isn't a bad thing. This is specific to the ru locale, and is a bit of a special case, but if this is how we want to do things then I'll + it.
Attachment #667350 - Flags: review?(kev) → review+
If we want to give a hint to the user about where the search is going, maybe we should figure a better solution to the "issue" than keeping datauris of search engine logos in the page (search engine definitions? search service util? remote hosting?). Yandex is not much different than Badoo or Yahoo, regarding penetration, keeping one means we should accept any request to add further, but the current implementation is not really meant for that.
That said, is not that I have strong feelings about this.
Removing the code now isn't really much of a win, and there's some downside to the partner builds. Seems like we can remove most of the downside by creating an alternative design that shows the engine's name as text rather than a logo, if none exists. I think that should be pretty straightforward, right? That will also benefit other situations where people have non-default defaults.
I agree that providing additional information about non-default default search engines to users is a good idea, but we're currently giving Yandex special treatment that no other non-default default receives, so I think removing that exception is still a valid change to make.

If we want to provide to provide metadata and/or logos for all of our non-default partners that have partner builds, I'm in support of that, and a followup bug could be filed for it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c194ae914a55
Target Milestone: --- → Firefox 18
I pasted the link above. Here's the correct one:
https://hg.mozilla.org/integration/mozilla-inbound/rev/331fae4331ba
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #8)
> Seems like we can remove most of the downside by
> creating an alternative design that shows the engine's name as text rather
> than a logo, if none exists.

Sort of a watermark... I wonder why we don't just set a placeholder text with the engine name?
https://hg.mozilla.org/mozilla-central/rev/331fae4331ba
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to Marco Bonardo [:mak] from comment #11)
> Sort of a watermark... I wonder why we don't just set a placeholder text
> with the engine name?

There's code in setupSearchEngine that tries to do that, but only if an image is specified, and it doesn't work because .name is undefined :/
Target Milestone: Firefox 18 → ---
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.