1822604 - Error when trying to place an SPOC topsite to the last spot with a new browser profile

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Nan Jiang [:nanj]]

With a new browser profile, when setting spoc-topsite-positions to 7, seeing the following error message:

JavaScript error: resource://activity-stream/lib/TopSitesFeed.jsm, line 928: TypeError: can't access property "show_sponsored_label", withPinned[index] is undefined

Offhand I think it's because we only have 7 organic tiles (i.e. default tiles), but it wanted to access withPinned[7] which would be undefined. We will need to consider those empty slots when injecting spoc tiles to the topsites array.

Comment 2

[Nan Jiang [:nanj]]

Attachment: Bug 1822604 - Fix an edge case for spoc topsites injection r=thecount

Comment 3

[Nan Jiang [:nanj]]

[Tracking Requested - why for this release]: this is a bugfix for a feature agains which we're going to run an experiment in release 112.

Comment 4

[Nan Jiang [:nanj]]

Attachment: Bug 1822604 - Fix an edge case for spoc topsites injection r=thecount

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77bb7e3dc44c
Fix an edge case for spoc topsites injection r=thecount

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/77bb7e3dc44c

Comment 7

[Nan Jiang [:nanj]]

Comment on attachment 9324098 [details]
Bug 1822604 - Fix an edge case for spoc topsites injection r=thecount

Beta/Release Uplift Approval Request

• User impact if declined: The experiment user group will not be able to dismiss certain topsites as expected
• 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: Please reference STR of Bug 1822347
• List of other uplifts needed: Bug 1822027
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch would only apply to some experiment users, we can turn off the experiment through Nimbus if needed.
• String changes made/needed: None
• Is Android affected?: No

Comment 8

[Nan Jiang [:nanj]]

Hi Carmen, could you also verify this fix using this STR? Thanks!

Comment 9

[Carmen Fat [:cfat] - Ecosystem QA]

I have verified this issue on the latest Firefox Nightly 113.0a1 (Build ID: 20230321213816), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.

• I used the STR from Bug 1822347 and can confirm that all the non-sponsored tiles can be dismissed correctly.
• I also verified that the dismissed tiles are not re-displayed at browser restart and in New Windows and new tabs.

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9324098 [details]
Bug 1822604 - Fix an edge case for spoc topsites injection r=thecount

Approved for 112.0b6

Comment 11

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/25e3b4ed0a59

Comment 12

[Carmen Fat [:cfat] - Ecosystem QA]

I have verified this issue on the latest Firefox Beta 112.0b6 (Build ID: 20230323181038), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.

• I used the STR from Bug 1822347 and can confirm that all the non-sponsored tiles can be dismissed correctly.
• I also verified that the dismissed tiles are not re-displayed at browser restart and in New Windows and new tabs.