Pocket newtab sometimes new profile has no Pocket stories
Categories
(Firefox :: New Tab Page, defect, P1)
Tracking
()
People
(Reporter: thecount, Assigned: thecount)
Details
Attachments
(1 file)
|
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
|
Details | Review |
This seems like it happens only sometimes, and it is often resolved on a refresh.
I think this is due to how we use region to setup stories. We need region to know if we use stories or not, and that can take a moment on first profile startup. We may know the region before we start setting up newtab, and we may not.
If we know, we can get ahead and start and everything works as expected.
If we don't know region yet, we setup newtab anyway, and listen for when the region is set. In this case, newtab is about to setup stories but is going to fail, then set the loaded flag. Once the region is ready, we then do another attempt to setup stories. This attempt however, checks that loaded is true. There is a case where region is ready late, and loaded is not ready yet, causing newtab to render a handful of empty stories.
To test, load a new profile and check the first newtab that is opened (you don't need to open it manually), do this a few times, I can usually hit it 1 out of 4.
If you refresh, the region should be ready now because there is a second cache update that should be ready for startup, but even that's not a guarantee to be ready before region, I've just never been able to trigger it. Just very unlikely.
Expected: should see the normal set of stories.
Actual: Empty boxes instead.
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 1•2 years ago
|
||
|
|
Assignee | |
Comment 2•2 years ago
|
||
I think was caused by bug 1858038, but I actually don't think bug 1858038 is wrong.
What I think is going on is bug 1858038 used to calculate showSpocs in the json Pocket config pref, we moved this to its own standalone pref.
The Pocket config pref would fire a generic update and the new spoc pref fires only a spoc update. This actually makes sense and I think is correct.
Pocket does a complete update on a generic update, because we don't know what about the config changed, so we just update everything.
This would update newtab enough and late enough to cover up the race condition for region, and cause it to almost always win in my testing.
With showSpocs moved into its own update, it still fires an update as often, but now we only update spocs, and not everything, thus this bug no longer gets covered.
I think the solution is to do an update earlier on pref change even if newtab is not loaded, so we don't miss the initial opportunity to do the update.
|
||
Updated•2 years ago
|
|
||
Comment 4•2 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 5•2 years ago
|
||
Comment on attachment 9360853 [details]
Bug 1861849 - Pocket newtab fixing startup issue for stories not displaying
Beta/Release Uplift Approval Request
- User impact if declined: New profiles can see no stories on first newtab load. A refresh fixes it, but that's likely not obvious to the user.
- 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: To test, load the browser with a new profile and check the first newtab that is opened (you don't need to open it manually), do this a few times, I can usually hit it 1 out of 4. I used --temp-profile to speed things along.
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): It's only 1 line of code and reasonably safe. It's been on nightly for a week, and the potential negative impact of this change is likely not worse than what the bug it is fixing is causing.
It's late in the beta cycle, but I feel like the above counter that and make this low.
- String changes made/needed: None
- Is Android affected?: No
|
|
Assignee | |
Updated•2 years ago
|
|
||
Comment 6•2 years ago
|
||
Comment on attachment 9360853 [details]
Bug 1861849 - Pocket newtab fixing startup issue for stories not displaying
Approved for 120.0b8
|
|
||
Updated•2 years ago
|
|
||
Updated•2 years ago
|
|
||
Comment 8•2 years ago
|
||
I have verified that this issue is no longer reproducible using the steps from comment 5 on the latest Firefox Nightly 121.0a1 (Build ID: 20231116093942) and latest Firefox Beta 120.0 (Build ID: 20231113165053) installed on Windows 10 x64, macOS 14.1.1, and Ubuntu 22.04 x64. I can confirm that the Pocket stories are successfully displayed on the first opened newtab.
Description
•