CFR can wrongly recommend outdated versions of the addons

VERIFIED FIXED in Firefox 63

Status

()

defect
P1
normal
VERIFIED FIXED
9 months ago
5 months ago

People

(Reporter: ppop, Assigned: ursula)

Tracking

(Blocks 1 bug)

Trunk
Firefox 64
Unspecified
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 unaffected, firefox63+ verified, firefox64 verified)

Details

Attachments

(4 attachments, 2 obsolete attachments)

Posted image addon.gif
[Affected versions]:
- Firefox Beta, Version 63.0b9, Build ID 20180924202103

[Affected Platforms]:
- All Windows
- All Linux
- All Mac

[Prerequisites]:
- Have the "browser.newtabpage.activity-stream.asrouter.messageProviders" pref set to "[{"id":"cfr", "cohort": "one_per_day", "frequency": {"custom": [{"period": "daily", "cap": 1}]}, "type":"local","localProvider":"CFRMessageProvider","enabled":true}]"
- Frecency for https://www.reddit.com is at least 10000.

[Steps to reproduce]:
1. Open the browser with the profile from prerequisites.
2. Open a new tab and navigate to https://www.reddit.com
3. Click the "Recommendation" button and click the "Add Now" button from the doorhanger.
4. Observe the doorhanger.

[Expected result]:
- The Add-on is downloaded and the option to enable it is displayed.

[Actual result]:
- The Add-on is not downloaded and the following error message is displayed: "The add-on could not be downloaded because of a connection failure."

[Additional Notes]:
- In the Browser Console the following error is displayed: 1537952045667 addons.xpi WARN Download of https://addons.mozilla.org/firefox/downloads/file/991623/reddit_enhancement_suite-5.12.5-an+fx.xpi failed: 404 Not Found
- The latest AMO version of the "Reddit Enhancement Suite" is 5.12.6 while the addon attempts to install version 5.12.5
- I have attached a screen recording of the issue:
Assignee

Updated

9 months ago
Assignee: nobody → usarracini
Priority: -- → P1
It sounds like in order to resolve this issue we will need change the strategy here to hit an external AMO api rather than hard-coding URLs, since the hard-coded URLs fail to install anything and we can't control the update cycle of a particular add-on.

Because this results in a failure to install the ad-don, this is a blocking bug for the 63 study.
Blocks: 1491379
Hi Kate, please land the patch today on mozilla-central and attach a patch for uplift if you want it in 63 beta 10 that will ship this Friday, thanks!
Flags: needinfo?(khudson)

Comment 4

9 months ago
Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/05fb100d324931609cc951fce101fe8535cd5e10
Fix Bug 1494275 - CFR can wrongly recommend outdated versions of the addons (#4451)

Updated

9 months ago
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Assignee

Comment 6

9 months ago
Comment on attachment 9012311 [details]
Bug 1494275 - Support dynamic install urls for CFR

Ursula Sarracini (:ursula) has approved the revision.
Attachment #9012311 - Flags: review+
Assignee

Comment 7

9 months ago
Yeah this was sort of anticipated since we're hard coding a url with the version in it. We didn't know it was going to break the entire experience, we thought it would just install the old version of the add-on, which wouldn't have been so bad. I will look into finding a better solution for this, and we can get this uplifted.

Worst case, we have to query the add-ons API to get the url to the latest xpi when we show the recommendation. This would involve making a network request when we show the recommendation. Not ideal
Iteration: --- → 64.2 (Sep 28)

Comment 8

9 months ago
Pushed by khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0595bcbe068
Support dynamic install urls for CFR r=ursula

Comment 9

9 months ago
Does anyone know how often the browser queries the AMO API endpoint?

From AMO's perspective, we'd like to know the estimated amount of requests (and the incoming traffic pattern, if possible) that will be introduced to our service because of this change, so that we can adjust our capacity accordingly.
Comment on attachment 9012311 [details]
Bug 1494275 - Support dynamic install urls for CFR

Approval Request Comment
[Feature/Bug causing the regression]: URLS for recommended add-ons no longer work if the add-on is updated  
[User impact if declined]: users will receive recommendations that fail if accepted
[Is this code covered by automated tests?]: yes 
[Has the fix been verified in Nightly?]: not yet, patch has landed on Mozilla central
[Needs manual test from QE? If yes, steps to reproduce]: see steps to reproduce in the first comment of this bug
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]:no 
[Why is the change risky/not risky?]: affects shield study only
[String changes made/needed]: none
Attachment #9012311 - Flags: approval-mozilla-beta?
Comment on attachment 9012311 [details]
Bug 1494275 - Support dynamic install urls for CFR

Blocker to CFR shield study launching on Oct.1st, approved for uplift to  63 beta 10. Thanks.
Flags: needinfo?(khudson)
Attachment #9012311 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi, I get this conflict when trying to graft this revision:

merging browser/components/newtab/content-src/asrouter/templates/CFR/templates/ExtensionDoorhanger.schema.json       
merging browser/components/newtab/lib/CFRMessageProvider.jsm                                                         
merging browser/components/newtab/lib/CFRPageActions.jsm                                                             
merging browser/components/newtab/test/unit/asrouter/templates/ExtensionDoorhanger.test.jsx                          
warning: conflicts while merging browser/components/newtab/lib/CFRMessageProvider.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/components/newtab/lib/CFRPageActions.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/components/newtab/test/unit/asrouter/templates/ExtensionDoorhanger.test.jsx!

ursula: Can you please have a look at this? Thank you!
Flags: needinfo?(usarracini)
We have a patch with merge conflicts fixed, we are just waiting on phabricator to publish. If it doesn't in the next couple of minutes we will attach as a text diff.
Flags: needinfo?(usarracini)
Assignee

Comment 15

9 months ago
Approval Request Comment
See previous request - this patch fixes the merge conflict
Attachment #9012593 - Flags: review?(khudson)
Attachment #9012593 - Flags: approval-mozilla-beta?
Attachment #9012593 - Flags: review?(khudson) → review+
Comment on attachment 9012593 [details] [diff] [review]
Bug_1494275_beta_uplift_patch.diff

Updated patch approved for uplift to beta.
Attachment #9012593 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi, I get the following conflict when trying to import this:

applying Bug_1494275.diff
file browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js already exists
1 out of 1 hunks FAILED -- saving rejects to file browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
Flags: needinfo?(usarracini)
Assignee

Comment 18

9 months ago
Approval Request Comment
Sorry this is the actual merge conflict fix. Last one!
Attachment #9012593 - Attachment is obsolete: true
Flags: needinfo?(usarracini)
Attachment #9012595 - Flags: review?(khudson)
Attachment #9012595 - Flags: approval-mozilla-beta?
Attachment #9012595 - Flags: review?(khudson) → review+
Attachment #9012614 - Attachment is obsolete: true
Attachment #9012595 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have verified that this issue is no longer reproducible using Firefox Beta (Version 63.0b10, Build ID 20180927143327) and Firefox Nightly (Version 64.0a1, Build ID 20180927220034) on Windows 10 x64, Mac 10.13 and Arch Linux.
Status: RESOLVED → VERIFIED

Updated

9 months ago
Blocks: 1494427

Comment 22

9 months ago
https://hg.mozilla.org/mozilla-central/rev/410ea4b30b37
Iteration: 64.2 (Sep 28) → 64.3 (Oct 12)
Target Milestone: --- → Firefox 64
Depends on: 1526387
You need to log in before you can comment on or make changes to this bug.