1649883 - Recommended by Pocket option disappears from setting after unchecking and restarting browser

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[kirayahiroz]

Attachment: Firefox78Pocket.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.116 Safari/537.36 Edg/83.0.478.56

Steps to reproduce:

1. Go to Firefox settings -> Home
2. Uncheck Recommended by Pocket
3. Restart browser
4. Go into Firefox settings -> Home

Actual results:

Recommended by Pocket is no longer an option to enable/disable and is completely gone

Expected results:

Recommended by Pocket should still be present, allowing the user to enable the option if they want in future.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Ed Lee :Mardak]

Looks like bug 1446276 made it so the system pref value defaults to false when the section value is toggled.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1446276

Comment 4

[Scott [:thecount] Downe]

Attachment: Bug 1649883 - Fix user pref for pocket toggle off.

Comment 5

[Scott [:thecount] Downe]

[Tracking Requested - why for this release]: Simple fix that actually removes code to revert to the a partial previous state.

Could impact UX if users disable pocket temporarily, it'll be hard to re enable.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c580de9a1129
Fix user pref for pocket toggle off. r=gvn

Comment 7

[Scott [:thecount] Downe]

Comment on attachment 9162251 [details]
Bug 1649883 - Fix user pref for pocket toggle off.

Beta/Release Uplift Approval Request

• User impact if declined: Could impact UX if users disable pocket temporarily, it'll be hard to re enable.

• Is this code covered by automated tests?: Yes

• Has the fix been verified in Nightly?: No

• Needs manual test from QE?: Yes

• If yes, steps to reproduce: Steps to reproduce:

  Go to Firefox settings -> Home
  Uncheck Recommended by Pocket
  Restart browser
  Go into Firefox settings -> Home

Actual results:

Recommended by Pocket is no longer an option to enable/disable and is completely gone

Expected results:

Recommended by Pocket should still be present, allowing the user to enable the option if they want in future.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We're just removing 7 lines of code that partially revert to an earlier state.
• String changes made/needed: none

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for failures and crashes on nsSocketTransport

backout: https://hg.mozilla.org/integration/autoland/rev/ef65d83ad74bbd353d83fbe256441ff0f6e503f3

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=ZHxmkMdHQ3CLtpPA0SWV8g.0&revision=c580de9a11297ebb5e971ac93c80f58508e5793a&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309189758&repo=autoland&lineNumber=1069

[task 2020-07-09T19:21:02.316Z] 19:21:02 INFO - FATAL ERROR: Non-local network connections are disabled and a connection attempt to getpocket.cdn.mozilla.net (23.56.13.14) was made.
[task 2020-07-09T19:21:02.316Z] 19:21:02 INFO - You should only access hostnames available via the test networking proxy (if running mochitests) or from a test-specific httpd.js server (if running xpcshell tests). Browser services should be disabled or redirected to a local server.
[task 2020-07-09T19:21:02.316Z] 19:21:02 INFO - Hit MOZ_CRASH(Attempting to connect to non-local address!) at /builds/worker/checkouts/gecko/netwerk/base/nsSocketTransport2.cpp:1318
[task 2020-07-09T19:21:02.375Z] 19:21:02 INFO - [Parent 1736, Main Thread] WARNING: Suboptimal indexes for the SQL statement 0x19d5f1f0 (http://mzl.la/1FuID0j).: file /builds/worker/checkouts/gecko/storage/mozStoragePrivateHelpers.cpp, line 106
[task 2020-07-09T19:21:02.559Z] 19:21:02 INFO - Initializing stack-fixing for the first stack frame, this may take a while...
[task 2020-07-09T19:21:20.540Z] 19:21:20 INFO - #01: mozilla::net::nsSocketTransport::OnSocketEvent(unsigned int, nsresult, nsISupports*) [netwerk/base/nsSocketTransport2.cpp:0]
[task 2020-07-09T19:21:20.540Z] 19:21:20 INFO - #02: mozilla::net::nsSocketEvent::Run() [netwerk/base/nsSocketTransport2.cpp:94]
[task 2020-07-09T19:21:20.540Z] 19:21:20 INFO - #03: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1236]
[task 2020-07-09T19:21:20.540Z] 19:21:20 INFO - #04: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:513]
[task 2020-07-09T19:21:20.541Z] 19:21:20 INFO - #05: mozilla::net::nsSocketTransportService::Run() [netwerk/base/nsSocketTransportService2.cpp:1195]
[task 2020-07-09T19:21:20.541Z] 19:21:20 INFO - #06: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1236]
[task 2020-07-09T19:21:20.541Z] 19:21:20 INFO - #07: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:513]
[task 2020-07-09T19:21:20.541Z] 19:21:20 INFO - #08: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:332]
[task 2020-07-09T19:21:20.541Z] 19:21:20 INFO - #09: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:334]
[task 2020-07-09T19:21:20.542Z] 19:21:20 INFO - #10: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:328]
[task 2020-07-09T19:21:20.542Z] 19:21:20 INFO - #11: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:310]
[task 2020-07-09T19:21:20.542Z] 19:21:20 INFO - #12: static nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:449]
[task 2020-07-09T19:21:20.897Z] 19:21:20 INFO - #13: _PR_NativeRunThread(void*) [nsprpub/pr/src/threads/combined/pruthr.c:399]
[task 2020-07-09T19:21:20.897Z] 19:21:20 INFO - #14: pr_root(void*) [nsprpub/pr/src/md/windows/w95thred.c:139]
[task 2020-07-09T19:21:20.897Z] 19:21:20 INFO - fix-stacks error: failed to read breakpad symbols dir Z:\task_1594319313\build\symbols\ucrtbase.DLL for Z:\task_1594319313\build\application\firefox\ucrtbase.DLL
[task 2020-07-09T19:21:20.897Z] 19:21:20 INFO - fix-stacks note: this is expected and harmless for system libraries on debug automation runs
[task 2020-07-09T19:21:20.897Z] 19:21:20 INFO - #15: o____lc_collate_cp_func [Z:\task_1594319313\build\application\firefox\ucrtbase.DLL + 0x3e16f]
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - fix-stacks error: failed to read breakpad symbols dir Z:\task_1594319313\build\symbols\kernel32.pdb for C:\windows\system32\kernel32.dll
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - fix-stacks note: this is expected and harmless for system libraries on debug automation runs
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - #16: BaseThreadInitThunk [C:\windows\system32\kernel32.dll + 0x4ef3c]
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - fix-stacks error: failed to read breakpad symbols dir Z:\task_1594319313\build\symbols\ntdll.pdb for C:\windows\SYSTEM32\ntdll.dll
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - fix-stacks note: this is expected and harmless for system libraries on debug automation runs
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - #17: RtlInitializeExceptionChain [C:\windows\SYSTEM32\ntdll.dll + 0x63618]
[task 2020-07-09T19:21:20.898Z] 19:21:20 INFO - #18: RtlInitializeExceptionChain [C:\windows\SYSTEM32\ntdll.dll + 0x635eb]
[task 2020-07-09T19:21:20.899Z] 19:21:20 INFO - Exiting due to channel error.
[task 2020-07-09T19:21:20.899Z] 19:21:20 INFO - [GPU 3972, Main Thread] WARNING: Shutting down GPU process early due to a crash!: file /builds/worker/checkouts/gecko/gfx/ipc/GPUParent.cpp, line 522
[task 2020-07-09T19:21:20.899Z] 19:21:20 INFO - Exiting due to channel error.
[task 2020-07-09T19:21:20.899Z] 19:21:20 INFO - Exiting due to channel error.
[task 2020-07-09T19:23:56.092Z] 19:23:56 ERROR - TEST-UNEXPECTED-FAIL | None | application terminated with exit code 1
[task 2020-07-09T19:23:56.092Z] 19:23:56 INFO - REFTEST INFO | Copy/paste: Z:/task_1594319313/fetches\minidump_stackwalk\minidump_stackwalk.exe c:\users\task_1594319313\appdata\local\temp\tmpe1kmuf.mozrunner\minidumps\c5b81f7e-e6d5-4576-bffe-e07222ee6c2d.dmp Z:\task_1594319313\build\symbols
[task 2020-07-09T19:24:19.264Z] 19:24:19 INFO - REFTEST INFO | Saved minidump as Z:\task_1594319313\build\blobber_upload_dir\c5b81f7e-e6d5-4576-bffe-e07222ee6c2d.dmp
[task 2020-07-09T19:24:19.271Z] 19:24:19 INFO - REFTEST INFO | Saved app info as Z:\task_1594319313\build\blobber_upload_dir\c5b81f7e-e6d5-4576-bffe-e07222ee6c2d.extra
[task 2020-07-09T19:24:19.512Z] 19:24:19 INFO - REFTEST PROCESS-CRASH | pid: None | application crashed [@ mozilla::net::nsSocketTransport::InitiateSocket()]

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Are we going to want to fix this on ESR78 also?

Comment 10

[Scott [:thecount] Downe]

Hey Ryan, sorry, I'm not sure what firefox-esr78 is.

I'm looking into the failures now.

Comment 11

[Tim Spurway [:tspurway]]

Scott, we will need a clean patch soon to make 79.

Comment 12

[Scott [:thecount] Downe]

Trying to land again.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3f15c378fbd
Fix user pref for pocket toggle off. r=gvn,remote-protocol-reviewers,perftest-reviewers,maja_zf

Comment 14

[Scott [:thecount] Downe]

Seeing how this got backed out once, and is late in the uplift process, I'm suggest this is too risky to uplift and we should instead let it ride the trains.

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9162251 [details]
Bug 1649883 - Fix user pref for pocket toggle off.

Per comment 14.

Comment 17

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (80.0a1 Build ID - 20200716212737) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.15. Now, the "Recommended by Pocket" option is still displayed after browser restart.

Comment 18

[Mike Kaply [:mkaply]]

I realize this is a year ago, but can someone explain this change?

The correct pref is browser.newtabpage.activity-stream.feeds.section.topstories. You can see that by looking at the source for preferences in the browser:

<checkbox xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" class="section-checkbox tail-with-learn-more" src="chrome://global/skin/icons/pocket.svg" data-l10n-id="home-prefs-recommended-by-header" data-l10n-args="{"provider":"Pocket"}" preference="browser.newtabpage.activity-stream.feeds.section.topstories" checked="true" label="Recommended by Pocket"><image class="checkbox-check" checked="true"/><hbox class="checkbox-label-box" flex="1"><image class="checkbox-icon" src="chrome://global/skin/icons/pocket.svg"/><label class="checkbox-label" flex="1">Recommended by Pocket</label></hbox></checkbox>

This broke the ability to disable Pocket in policy because it's flipping the wrong pref.

What is browser.newtabpage.activity-stream.feeds.system.topstories?

This was just now found because folks are trying to disable Pocket on the ESR. I'm looking into why our tests didn't kick in here.

Comment 19

[Scott [:thecount] Downe]

Hey, sorry about this, thought we ran this through policy/enterprise at the time, but it was so long ago, and looks like something got missed.

I'll try to explain the prefs as concisely as I can.

Certain regions or locales don't support Pocket for a number of reasons. This includes the ability to turn it off and on at a user level.

This means regions changing via travel could flip the Pocket pref on or off, depending. Also caused by language packs being installed on uninstalled.

If this happened it could also reset the previous state of Pocket, either turning it back on when it was initially off, or off it was initially on, just because they travelled.

The solution we ended up with was a two pref system, where 1 pref was a user set pref, and the other a system set pref.
browser.newtabpage.activity-stream.feeds.section.topstories and browser.newtabpage.activity-stream.feeds.system.topstories

Both prefs shouls turn off Pocket, but section pref normally allows the user to turn it back on via a user setting, and system pref turns it off at a higher level.

Bulk of that work happened in bug 1446276