Closed Bug 1528059 Opened 2 years ago Closed 2 years ago

Dismissed Pocket stories come back until stories update


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




Firefox 67
67.2 - Feb 11 - 24
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed


(Reporter: Mardak, Assigned: thecount)



(Keywords: github-merged, regression)


(1 file)

Looks like TopStoriesFeed only persists removing blocked urls when it processes a new set of stories:

This means if a user dismisses a story, it'll disappear immediately thanks to Reducer but then I'm guessing on SYSTEM_TICK 5 minutes later, the story comes back because it's probably in TopStoriesFeed.stories.

Only after a story update (every 30 minutes) will it be filtered out for good.

Iteration: --- → 67.2 - Feb 11 - 24
Priority: -- → P1

I happened to have 64.0.2 release and a quick test shows the dismissed article coming back after waiting 5 minutes. So this isn't a recent regression

I believe this regressed with bug 1479458 since Firefox 63. It added a doContentUpdate() in SYSTEM_TICK:

And it reads the stories from this.stories:

… instead of accessing the redux state that has stories removed, e.g., => === SECTION_ID)
Blocks: 1479458
Flags: needinfo?(sdowne)

Good find. Yup I think you're right about this being a regression because of doContentUpdate.

I'm not sure if what's in state is usable as a replacement to this.stories. I think redux does modification on it (spocs and deduping). Either way, it makes me nervous that we might start seeing 2 of the same spocs side by side by doing this. We generally only use store in specific cases in this file because of how much redux can change stories, iirc, but I can confirm.

Another options is, doContentUpdate just didn't send stories unless fetchStories updated something, because this.stories gets properly transform'd inside of fetchStories, it should have the blocked list.

An easier but less optimal option is to always run this.stories through a .filter(s => !NewTabUtils.blockedLinks.isBlocked({"url": s.url})) before doContentUpdate in system tick.

Flags: needinfo?(sdowne)
Flags: needinfo?(edilee)
Assignee: nobody → sdowne

Redux state doesn't update main for spoc stuff as TopStoriesFeed uses OnlyToOneContent:

… where that action creator explicitly says to skipMain:

So the simplest fix should be just changing updateProps.rows = this.stories to use => === SECTION_ID).rows

There's an existing handler for link blocks that filters out spocs, so that could be reused to filter out this.stories instead of iterating over all blocked pages (which aren't limited to just stories) on every update.

Flags: needinfo?(edilee)
Blocks: 1532321
Closed: 2 years ago
Keywords: github-merged
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.