1522810 - TAAR - personalized recommendations appear in private window

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P1)

Description

[Vlad Jiman]

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:

1. 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%

2. Go to about:addons and select the Get Add-ons tab, notice the recommended extensions listed here.

3. Open a new Private Browsing window and go to the about:addons again

4. 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.

Comment 1

[Vlad Jiman]

Attachment: Recommendations.png

Comment 2

[Stuart Colville [:muffinresearch]]

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.

Comment 3

[Shane Caraveo (:mixedpuppy)]

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.

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1522810 disable client id header in private window, r?aswan

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.

Comment 5

[:shell escalante]

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.

Comment 6

[Shane Caraveo (:mixedpuppy)]

I'm currently working on an automated test for this patch, will update soon as I can.

Comment 7

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1edc49862fcd7a8ae6fbc10856880e5e050b25c7

Comment 8

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3646ecaeed7e20a4fa7539f209afbd5a59f8e6d2

Comment 9

[Shane Caraveo (:mixedpuppy)]

patch is finished, comment 7 is for 66, comment 8 is for beta 65. Once builds are done there QA can probably use those to verify.

Patch has test and applies to both m-c and beta.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab23c51b25a3
disable client id header in private window, r=aswan

Comment 11

[Shane Caraveo (:mixedpuppy)]

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

Comment 12

[Shane Caraveo (:mixedpuppy)]

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

[Stuart Colville [:muffinresearch]]

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.

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/ab23c51b25a3

Comment 15

[Julien Cristau [:jcristau]]

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)

Comment 16

[Vlad Jiman]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/1286c5851845

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 20

[Shane Caraveo (:mixedpuppy)]

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.

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks Shane for the quick reply. Good to know that, in the future, this bug will get caught via automated tests.

Comment 22

[Vlad Jiman]

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.