The default bug view has changed. See this FAQ.

Add the extensions.getAddons.getWithPerformance.url to mobile default prefs

RESOLVED FIXED in Firefox 11

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: gps)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 2 obsolete attachments)

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.
status-firefox11: --- → affected
status-firefox12: --- → affected
(Assignee)

Comment 1

5 years ago
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!
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #593639 - Flags: review?(bmcbride)
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.)
Attachment #593639 - Flags: review?(bmcbride) → review-
(Assignee)

Comment 3

5 years ago
(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*
(Assignee)

Comment 4

5 years ago
Created attachment 593691 [details] [diff] [review]
Preference fix-up, v2

Removed clown shoes.
Attachment #593639 - Attachment is obsolete: true
Attachment #593691 - Flags: review?(bmcbride)
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
Attachment #593691 - Flags: review?(bmcbride) → review-
(Assignee)

Comment 6

5 years ago
Created attachment 593695 [details] [diff] [review]
Preference fix-up, v3

Remove other clown shoe.
Attachment #593691 - Attachment is obsolete: true
Attachment #593695 - Flags: review?(bmcbride)
Attachment #593695 - Flags: review?(bmcbride) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9689287ce87
Whiteboard: [inbound]
(Assignee)

Comment 8

5 years ago
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.
Attachment #593695 - Flags: approval-mozilla-beta?
Attachment #593695 - Flags: approval-mozilla-aurora?

Comment 9

5 years ago
See https://bugzilla.mozilla.org/show_bug.cgi?id=716736#c16. Let's wait a couple of more days before approving.
https://hg.mozilla.org/mozilla-central/rev/a9689287ce87
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Depends on bug716736 for uplift
Also, the URL for android uses "mobile" and this is changed in bug 715262
Comment on attachment 593695 [details] [diff] [review]
Preference fix-up, v3

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.
Attachment #593695 - Flags: approval-mozilla-beta?
Attachment #593695 - Flags: approval-mozilla-beta+
Attachment #593695 - Flags: approval-mozilla-aurora?
Attachment #593695 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 14

5 years ago
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.
status-firefox11: affected → fixed
status-firefox12: affected → fixed
You need to log in before you can comment on or make changes to this bug.