1676185 - Pinning topsite moves the tile with + x position when sponsored topsites are active

Status: RESOLVED FIXED

(Firefox :: Top Sites, defect, P1)

Description

[Pablo]

Attachment: BUG2.jpg

[Suggested Severity:]

S3

[Description:]

Pinning a non-sponsored topsite will move the topsites from the current position.

[Environment:]

Windows 10
83.0b10-trybuild

[Preconditions:]

There are sponsored tiles listed in the topsites.

[Steps:]

1. Download and install RO localized Firefox.
2. Create an user.js and set: browser.topsites.useRemoteSetting = true and browser.search.region to "RO", connecting to the staging main-preview.
3. Start the browser with the predefined profile.
4. Open a new about:preferences#home and set topsites to be shown on 4 rows.
5. Open a new tab.
6. Pick any sponsored or non-sponsored topsite and choose to pin it using the menu.

[Actual Result:]

The topsite is pinned. The topsite is moved from the current position to position +x, where x represents the number of sponsored topsites in front of it.

[Expected Result:]

The topsite is pinned in the current position.

[Note:]

The behavior doesn’t reproduce if the sponsored top-sites are disabled. (browser.newtabpage.activity-stream.showSponsoredTopSites = false)

SEE SCREENSHOT ATTACHED, WARNER MOVED POSITION WHEN PINNED it also loosed its image.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1676185 - Make pinning from the site menu work the same as pinning by drag & drop. r=mikedeboer

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfe322864a91
Make pinning from the site menu work the same as pinning by drag & drop. r=mikedeboer

Comment 3

[Dorel Luca [:dluca]]

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

Comment 4

[Adrian Florinescu [:aflorinescu]]

Gave this a spin on 84.0a1 2020-11-12 / Windows 10 & Ubuntu 18.04 and at first glance it seems fixed, but I feel like a more deeper verification is required here. Leaving a reminder to reiterate on this one next round of testing.

Comment 5

[Adrian Florinescu [:aflorinescu]]

I'm not entirely sure why I've left this unverified at the time. Pretty sure this was covered as part of feature testing post comment#4. Nothing actionable here right now, removing qe+. For good measure, I've done a bit of smoke testing for this area with comment#0 STR on 137.0a1, all good