Last Comment Bug 723323 - Add the extensions.getAddons.getWithPerformance.url to mobile default prefs
: Add the extensions.getAddons.getWithPerformance.url to mobile default prefs
Status: RESOLVED FIXED
[inbound]
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Gregory Szorc [:gps]
:
Mentors:
Depends on: 716736
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-01 15:27 PST by Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
Modified: 2012-02-06 10:22 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Preference fix-up (4.00 KB, patch)
2012-02-01 16:09 PST, Gregory Szorc [:gps]
bmcbride: review-
Details | Diff | Review
Preference fix-up, v2 (4.00 KB, patch)
2012-02-01 18:11 PST, Gregory Szorc [:gps]
bmcbride: review-
Details | Diff | Review
Preference fix-up, v3 (3.99 KB, patch)
2012-02-01 18:20 PST, Gregory Szorc [:gps]
bmcbride: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-02-01 15:27:12 PST
To make it so we only send performance data to AMO when doing the background update check, bug 716736 added a new preference: extensions.getAddons.getWithPerformance.url

It's the same as extensions.getAddons.getWithPerformance.url, but with the performance query parameters added (those that were originally removed by bug 682356).

I'm assuming AMO tracks this data for mobile, but I don't know for sure.

Note that bug 682356 removed this in Fx11, and bug 716736 should hopefully be adding it back (for background update checks only) for Fx11. If we want no gap in the data for mobile (like desktop), this pref will need added for 11 (beta) for mobile too.
Comment 1 Gregory Szorc [:gps] 2012-02-01 16:09:03 PST
Created attachment 593639 [details] [diff] [review]
Preference fix-up

Seems pretty straight-forward. Looks like we didn't remove all references to performance parameters when they were ripped out a few months ago. Whoops!
Comment 2 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-02-01 18:07:05 PST
Comment on attachment 593639 [details] [diff] [review]
Preference fix-up

Review of attachment 593639 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ +231,5 @@
>  pref("extensions.getAddons.search.browseURL", "https://addons.mozilla.org/%LOCALE%/mobile/search?q=%TERMS%");
>  pref("extensions.getAddons.search.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%/%COMPATIBILITY_MODE%");
>  pref("extensions.getAddons.browseAddons", "https://addons.mozilla.org/%LOCALE%/mobile/");
> +pref("extensions.getAddons.get.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=mobile&appOS=%OS%&appVersion=%VERSION%");
> +pref("extensions.getAddons.getWithPerformance.url", "https://services.addons.mozilla.org/%LOCALE%/firefox/api/%API_VERSION%/search/guid:%IDS%?src=firefox&appOS=%OS%&appVersion=%VERSION%&tMain=%TIME_MAIN%&tFirstPaint=%TIME_FIRST_PAINT%&tSessionRestored=%TIME_SESSION_RESTORED%");

New pref is using /firefox/ instead of /mobile/, and src=firefox instead of src=mobile.

(Same goes for the other file.)
Comment 3 Gregory Szorc [:gps] 2012-02-01 18:08:41 PST
(In reply to Blair McBride (:Unfocused) from comment #2)
> New pref is using /firefox/ instead of /mobile/, and src=firefox instead of
> src=mobile.

*facepalm*
Comment 4 Gregory Szorc [:gps] 2012-02-01 18:11:15 PST
Created attachment 593691 [details] [diff] [review]
Preference fix-up, v2

Removed clown shoes.
Comment 5 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2012-02-01 18:17:21 PST
Comment on attachment 593691 [details] [diff] [review]
Preference fix-up, v2

Review of attachment 593691 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ +231,5 @@
>  pref("extensions.getAddons.search.browseURL", "https://addons.mozilla.org/%LOCALE%/mobile/search?q=%TERMS%");
>  pref("extensions.getAddons.search.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%/%COMPATIBILITY_MODE%");
>  pref("extensions.getAddons.browseAddons", "https://addons.mozilla.org/%LOCALE%/mobile/");
> +pref("extensions.getAddons.get.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=mobile&appOS=%OS%&appVersion=%VERSION%");
> +pref("extensions.getAddons.getWithPerformance.url", "https://services.addons.mozilla.org/%LOCALE%/mobile/api/%API_VERSION%/search/guid:%IDS%?src=firefox&appOS=%OS%&appVersion=%VERSION%&tMain=%TIME_MAIN%&tFirstPaint=%TIME_FIRST_PAINT%&tSessionRestored=%TIME_SESSION_RESTORED%");

Still one clownshoe on ;) src=firefox should be src=mobile
Comment 6 Gregory Szorc [:gps] 2012-02-01 18:20:48 PST
Created attachment 593695 [details] [diff] [review]
Preference fix-up, v3

Remove other clown shoe.
Comment 8 Gregory Szorc [:gps] 2012-02-01 19:55:00 PST
Comment on attachment 593695 [details] [diff] [review]
Preference fix-up, v3

[Approval Request Comment]

This patch goes hand in hand with bug 716736 and they should be treated as a unit. See approval comments on that bug.
Comment 9 Alex Keybl [:akeybl] 2012-02-02 06:54:19 PST
See https://bugzilla.mozilla.org/show_bug.cgi?id=716736#c16. Let's wait a couple of more days before approving.
Comment 10 Ed Morley [:emorley] 2012-02-02 06:54:39 PST
https://hg.mozilla.org/mozilla-central/rev/a9689287ce87
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2012-02-03 12:08:10 PST
Depends on bug716736 for uplift
Comment 12 Mark Finkle (:mfinkle) (use needinfo?) 2012-02-03 12:09:43 PST
Also, the URL for android uses "mobile" and this is changed in bug 715262
Comment 13 Alex Keybl [:akeybl] 2012-02-05 14:01:39 PST
Comment on attachment 593695 [details] [diff] [review]
Preference fix-up, v3

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.
Comment 14 Gregory Szorc [:gps] 2012-02-06 10:22:21 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/b577cc1e8d9b
https://hg.mozilla.org/releases/mozilla-beta/rev/6c764496e613

Patch did not apply cleanly on beta for some weird reason. The change was trivial, so I just copied the lines from tip. I have no clue why Mercurial failed to apply it. Who knows.

Note You need to log in before you can comment on or make changes to this bug.