Open Bug 1422867 Opened 7 years ago Updated 2 years ago

Wrong number of Top Sites displayed

Categories

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

defect

Tracking

()

Tracking Status
firefox59 --- wontfix
firefox60 --- wontfix

People

(Reporter: andreio, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [AS61MVP])

I get 11 topsites + 1 empty placeholder although I have a rich enough history + pinned sites. There is an issue with the way we filter through the pinned topsites (which have undefined entries in order to store position). `getLinksWithDefaults` method returns an array of length 12 but upon inspection it has 11 elements inside. Debugging the issue I have it tracked down to: https://github.com/mozilla/activity-stream/blob/master/system-addon/lib/TopSitesFeed.jsm#L129 This information is based on my profile > withPinned.length // 12 > checkedAdult.length // 10 > pinned.length // 15 Array [ null, null, null, null, null, null, null, null, null, null, 5 more… ] first 9 are undefined them the rest are defined The result is an array of length 12 with only 11 elements.
Maybe fix would be to just filter out null elements before we slice? Perma-link for previous comment. https://github.com/mozilla/activity-stream/blob/ce92c43ba20c2f2cc7b18b75ab1b2de3eeab84f9/system-addon/lib/TopSitesFeed.jsm#L129
Iteration: --- → 60.4 - Mar 12
Priority: P3 → P2
Whiteboard: [AS60MVP]
Blocks: 1437659
Iteration: 60.4 - Mar 12 → ---
Priority: P2 → P3
Iteration: --- → 61.1 - Mar 26
Should we just close this? When initially submitted you could not * Drag and drop topsites * Set a topsite in place of an empty placeholder I think these two features do a lot of good mitigating the issue. The source for the bug was switching from tiles to AS where you could have pinned links overflowing the AS topsites limit. I don't think it's likely for users to run into this anymore.
Whiteboard: [AS60MVP] → [AS61MVP]
I believe the underlying issue is that there are pinned sites that are not visible yet are deduping sites that could have been shown. There have been recent changes to drag-n-drop but that probably actually makes things worse in that you can drag sites to the second row. And along with the wider layout changes, we switched to a single row, which means there's 8 potentially out of view pinned sites. Being able to set a top site in an empty placeholder could help the user fill in some site, but if the user doesn't actively do something, it will look like there's missing top sites. Although looking at the screenshots of the original issues, it looks like there isn't even a placeholder and just an empty spot?
Iteration: 61.1 - Mar 26 → ---
Iteration: --- → 61.2 - Apr 9
Priority: P3 → P2
Iteration: 61.2 - Apr 9 → 61.4 - May 7
Iteration: 61.4 - May 7 → 62.1 - May 21
Priority: P2 → P3
Iteration: 62.1 - May 21 → 63.1 - July 9
Iteration: 63.1 - July 9 → ---
Component: Activity Streams: Newtab → New Tab Page
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.