TAAR - personalized recommendations appear in private window
Categories
(WebExtensions :: General, defect, P1)
Tracking
(firefox64 unaffected, firefox65+ verified, firefox66+ verified)
Tracking | Status | |
---|---|---|
firefox64 | --- | unaffected |
firefox65 | + | verified |
firefox66 | + | verified |
People
(Reporter: vlad.jiman, Assigned: mixedpuppy)
References
Details
(Whiteboard: [qa-triaged])
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-release+
|
Details |
There seem to be some issues affecting the recommendations shown in about:addons, while in Private Browsing Mode we do not get the default extensions listed and also the 'Allow Nightly to make personalized extension recommendations' does not seem to affect the recommendations in any way.
In order to reproduce the issue:
-
Open about:config using any profile and set the 'extensions.webservice.discoverURL' pref to point to the dev environment: https://discovery.addons-dev.allizom.org/%LOCALE%/firefox/discovery/pane/%VERSION%/%OS%/%COMPATIBILITY_MODE%
-
Go to about:addons and select the Get Add-ons tab, notice the recommended extensions listed here.
-
Open a new Private Browsing window and go to the about:addons again
-
Go to options and uncheck the 'Allow Nightly to make personalized extension recommendations' checkbox located in Privacy and Security, then go back to about:addons and see the recommendations.
Expected results:
Step 3, 4 - The default recommended addons should be shown instead of personalized ones
Actual results:
We are shown the same list of recommendations regardless of the checkbox or private browsing mode. In some remote cases while in private browsing mode, I've also had recommendations change upon refreshing the page.
Reporter | ||
Comment 1•6 years ago
|
||
Reporter | ||
Updated•6 years ago
|
Can you also reproduce this for production? We've enabled the addons-server side to return recommendations in order to test it, it would be good to know if it can also be reproduced there too.
Assignee | ||
Comment 3•6 years ago
|
||
I think I know what is happening here assuming I understand the bug: we are showing personalized recommendations in private browsing and we should not be doing that.
In bug 1489531 when I did the cookie implementation, it specifically uses userContextId to prevent the cookies being used in private browsing.
When we switched to the headers, I did not check for private browsing. So the header is being set regardless.
Assignee | ||
Comment 4•6 years ago
|
||
When switching to using a header for the discover pane I forgot
to check for privateness of the window. This patch should apply to both
m-c and beta.
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 5•6 years ago
|
||
ryanVM set the bug to tracking+ (so this fix will be in line for any dot release of 65).
Shane, Once the patch is ready for uplift, please nominate this fix for beta and release approval.
Stuart disabled offering recommendations on the server side for now. So it won't be live in 65 - until this lands. Then Stuart will turn it live server side again.
Assignee | ||
Comment 6•6 years ago
|
||
I'm currently working on an automated test for this patch, will update soon as I can.
Assignee | ||
Comment 7•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
Assignee | ||
Comment 9•6 years ago
|
||
Comment 10•6 years ago
|
||
Assignee | ||
Comment 11•6 years ago
|
||
Comment on attachment 9039127 [details]
Bug 1522810 disable client id header in private window, r?aswan
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
None
User impact if declined
personalized recommendations would appear in private browsing
Is this code covered by automated tests?
Yes
Has the fix been verified in Nightly?
No
Needs manual test from QE?
No
If yes, steps to reproduce
covered by test, but manual str if desired would be to check for the header via network monitor on a private window.
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
minimal change
String changes made/needed
none
Assignee | ||
Comment 12•6 years ago
|
||
Comment on attachment 9039127 [details]
Bug 1522810 disable client id header in private window, r?aswan
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
None
User impact if declined
see comment 11
Is this code covered by automated tests?
Yes
Has the fix been verified in Nightly?
Yes
Needs manual test from QE?
Yes
If yes, steps to reproduce
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
String changes made/needed
Comment 13•6 years ago
|
||
To prevent recommendations whilst this bug exists we've turned this off recommendations on amo production. To test this please use stage instead.
To point to stage you can set extensions.webservice.discoverURL to https://discovery.addons.allizom.org/%LOCALE%/firefox/discovery/pane/%VERSION%/%OS%/%COMPATIBILITY_MODE% in about:config.
Assignee | ||
Updated•6 years ago
|
Comment 14•6 years ago
|
||
bugherder |
Comment 15•6 years ago
|
||
Comment on attachment 9039127 [details]
Bug 1522810 disable client id header in private window, r?aswan
clearing beta approval, this is already in beta (66)
Reporter | ||
Comment 16•6 years ago
|
||
Verified using Nightly 66 and build 65 from comment #8. The Moz-Client-Id header is no longer showing while in private browsing mode, also default recommendations are shown while in Private browsing or if the 'Allow Nightly to make personalized extension recommendations' is unchecked.
Updated•6 years ago
|
Comment 17•6 years ago
|
||
Comment on attachment 9039127 [details]
Bug 1522810 disable client id header in private window, r?aswan
[Triage Comment]
TAAR blocker, approved for 65.0.1.
Comment 18•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Updated•6 years ago
|
Hi David, Shane, do we have automated unit tests for this? If this is covered by automated tests (https://bugzilla.mozilla.org/show_bug.cgi?id=1522810#c12) how did we not catch this regression sooner? Just wondering.
Assignee | ||
Comment 20•6 years ago
|
||
The original patch worked fine, but did not consider private windows (ie. it included rather than excluded private windows). Since private windows were not considered in the original patch, the test would not have either. The tests added in the patch here tests both cases.
Thanks Shane for the quick reply. Good to know that, in the future, this bug will get caught via automated tests.
Reporter | ||
Comment 22•6 years ago
|
||
Verified the fix using version 65.0.1, there don't appear to be any issues and I will mark fx 65 as verified as well. Testing was done using Windows 10 x64 bit using both new profiles, used profiles and the recommendations appear to be working as expected.
Description
•