Closed Bug 1001566 Opened 7 years ago Closed 7 years ago
Yahoo search plugin should use nresults param for smaller requests
There's other stuff we can use (see bug 1001528 for an explanation), but for mobile we should adopt nresults right now, since we only use the first four results, and that makes the request half the size on average. I'm going to make this a MozParam so we can share between plugins/tweak the value centrally. Note that this also cleans up the search plugin to use a properly parameterized search suggestion URL, which will made testing easier (i.e. how we've done desktop in bug 998071 ) Note that this patch is untested because I still don't have a working Android build VM. Maybe next week!
Attachment #8412783 - Flags: review?(margaret.leibovic)
Comment on attachment 8412783 [details] [diff] [review] limitYahooResultsAndroid Review of attachment 8412783 [details] [diff] [review]: ----------------------------------------------------------------- I tested this patch, and it works. ::: mobile/android/app/mobile.js @@ +239,5 @@ > > // pointer to the default engine name > pref("browser.search.defaultenginename", "chrome://browser/locale/region.properties"); > +// maximum number of search suggestions, as a string because the search service expects a string pref > +pref("browser.search.param.maxSuggestions", "4") You're missing a semicolon. You need to add that for this patch to work. @@ -523,5 @@ > -#ifdef MOZ_OFFICIAL_BRANDING > -pref("browser.search.param.yahoo-fr", "moz35"); > -pref("browser.search.param.yahoo-fr-cjkt", "moz35"); > -pref("browser.search.param.yahoo-fr-ja", "mozff"); > -#endif What were these bits used for before?
Attachment #8412783 - Flags: review?(margaret.leibovic) → review+
Those removed bits were the desktop codes, which we just don't use now (we've got mozilla_mobile_search everywhere)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Mike, is this bug going to land on other branches? Currently is only on central. Asking to be sure it's safe to update l10n searchplugin on Aurora with these settings.
Comment on attachment 8412783 [details] [diff] [review] limitYahooResultsAndroid Good point, I'll ask for uplift so we can roll it in. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: unnecessary data usage Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): very minimal String or IDL/UUID changes made by this patch: n/a Uplifting this will allow us to apply this change across Yahoo plugins (which get updated on Aurora) as a part of a larger update to those plugins. Should be very very safe.
Attachment #8412783 - Flags: approval-mozilla-aurora?
Attachment #8412783 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.