1469823 - A new Top Sites tile does not appear after deleting a tile

Status: VERIFIED FIXED

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

Description

[Patriciu Pop [:ppop], Ecosystem QA]

Attachment: empty_tile.gif

[Affected versions]:
- Nightly v62.0a1, Build ID 20180619220118

[Affected Platforms]:
- Windows 10 x64

[Prerequisites]:
Have enough browsing history so that the Top Sites section is populated.

[Steps to reproduce]:
1. Open the browser with the profile from prerequisites and open a new tab.
2. Hover one of the Top Sites tiles and click the "(...)" button.
3. Click the "Delete from History" option.
4. Click on "Delete From History" button.
5. Observe the Top Sites tiles.

[Expected results]:
- The next tile in queue appears in the last position.

[Actual results]:
- The tile is not replaced, instead it remains empty.

[Regression Range]:
73:29.10 INFO: Last good revision: 486bd3917deebcf6e1120859b111d200142d444d
73:29.10 INFO: First bad revision: c9ae7e47a6eff9aef6a691c638754d2f176ee0ad
73:29.10 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=486bd3917deebcf6e1120859b111d200142d444d&tochange=c9ae7e47a6eff9aef6a691c638754d2f176ee0ad

It seems that bug 1466971 has caused this behavior.

[Notes]:
- The tiles update and change correctly when dismissing the tile.
- Attached a screen recording of the issue.

Comment 1

[Andrei Oprea [:andreio]]

I don't think the last good revision is correct. I tried it and the bug still reproduces. Can you please confirm?

Comment 2

[Andrei Oprea [:andreio]]

The first good revision for me is before https://bugzilla.mozilla.org/show_bug.cgi?id=1462427 landed.

Comment 3

[Andrei Oprea [:andreio]]

I managed to track it down to https://github.com/mozilla/activity-stream/pull/4184

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/3e29130aadfc92d10488fba3bf831367c80bac24
fix(topsites): Listen for PLACES_LINK_DELETED actions to refresh (#4201)

Fix Bug 1469823 - A new Top Sites tile does not appear after deleting a tile

Comment 5

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/9dcaad7a978f

Comment 6

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the issue is no longer reproducible on the latest Nightly build (63.0a1, Build ID 20180627100027) on Windows 10 x64, Mac 10.13.5 and Ubuntu 16.04 x64.

@Ed this issue is still present in the Beta channel, should we consider uplifting this fix given that it is a regression?

Comment 7

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: Functionality regressed in Nightly 62 although interaction rate with "delete from history" for a top site probably isn't that high and the problem fixes itself

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift? I think it'd be good to fix in beta. (Somehow, I can easily imagine top sites people might want to delete. )

Comment 9

[Andrei Oprea [:andreio]]

Attachment: Bug 1469823 - Update Activity Stream TopSites after deleting a tile.

Review commit: https://reviewboard.mozilla.org/r/254230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254230/

Comment 10

[Andrei Oprea [:andreio]]

Comment on attachment 8989162 [details]
Bug 1469823 - Update Activity Stream TopSites after deleting a tile.

Approval Request Comment
[Feature/Bug causing the regression]: Regression tracked to https://github.com/mozilla/activity-stream/pull/4184
[User impact if declined]: UI behavior inconsistency
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes https://bugzilla.mozilla.org/show_bug.cgi?id=1469823#c6
[Needs manual test from QE? If yes, steps to reproduce]: Already checked 
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: No
[Why is the change risky/not risky?]: Small patch already tested in Nightly
[String changes made/needed]: n/a

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8989162 [details]
Bug 1469823 - Update Activity Stream TopSites after deleting a tile.

Verified fix for a recent regression, let's uplift for beta 5. 
Ed, I assume this was reviewed in github already since it landed in nightly already.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This has a merge conflict:

<<<<<<< local
    <em:version>2018.06.29.1011-6fefff6e</em:version>
=======
    <em:version>2018.06.22.1337-8d599e17</em:version>
>>>>>>> graft

Comment 13

[Ed Lee :Mardak]

Yes, it was reviewed and landed for nightly/master.

andreio, the commit needs to be uplifted first to our git branch for the version commit ids to make sense.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

OK, it can wait for beta 6, no rush to get this into beta 5 today.

Comment 15

[Ed Lee :Mardak]

Attachment: Bug 1469823 - A new Top Sites tile does not appear after deleting a tile.

Review commit: https://reviewboard.mozilla.org/r/254326/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254326/

Comment 16

[Ed Lee :Mardak]

Comment on attachment 8989276 [details]
Bug 1469823 - A new Top Sites tile does not appear after deleting a tile.

This was previously reviewed by me in comment 4 and approved in comment 11.

Comment 17

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/releases/mozilla-beta/rev/42ed67641f89

Comment 18

[Ed Lee :Mardak]

Comment on attachment 8989162 [details]
Bug 1469823 - Update Activity Stream TopSites after deleting a tile.

https://reviewboard.mozilla.org/r/254230/#review262044

Mostly just clearing the request as this was already reviewed and already landed. ;)

Comment 19

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to reproduce the initial issue on 62.0a1 (2018-06-21). I also can confirm that 62.0b6 build1 (20180705151535) is verified fixed across platforms (Windows 10 x64, Ubuntu 18.04 x64, macOS 10.12.6).