1470211 - Dismissing a "Bookmarked" card that is also "Saved to Pocket" causes the saved one to disappear after saving another website to Pocket

Status: VERIFIED FIXED

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

Description

[Patriciu Pop [:ppop], Ecosystem QA]

Attachment: AS-Highlights.gif

[Affected versions]:
-Nightly Version 62.0a1, Build ID 20180621013659
-Beta Version 61.0, Build ID 20180620142108
-Firefox Version 60.0.2, Build ID 20180605171542

[Affected Platforms]:
-Windows 10 x64
-Mac 10.13.5

[Steps to reproduce]:
1. Open the browser and access a website.
2. Save the page to Pocket and Bookmark it.
3. Open a new tab and dismiss the previously bookmarked card.
4. Navigate to a different website.
5. Save the page to Pocket and Bookmark it.
6. Open a new tab and observe the highlights section.

[Expected results]:
- The section correctly displays the first webpage saved to Pocket as well as the second webpage bookmarked and saved to Pocket.

[Actual results]:
- The first Pocket saved website is no longer displayed and only the second bookmarked and Pocket saved website is.

[Notes]:
- This issue is not reproducible on Ubuntu.
- Attached a screen recording of the issue.

Comment 2

[Ursula Sarracini (:ursula)]

This bug will have to land in 2 parts, one change will be made in NewTabUtils.jsm to check the open_url when processing highlights and filtering out blocked ones, and the other change will be to use the open_url to block the site. The second change will land in Activity Stream as it is part of the content code of Activity Stream

Comment 3

[Ursula Sarracini (:ursula)]

Attachment: Bug 1470211 - Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1

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

Comment 4

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4275

Comment 5

[Ursula Sarracini (:ursula)]

Comment on attachment 8996426 [details]
Bug 1470211 - Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260526/diff/1-2/

Comment 6

[[github robot]]

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

https://github.com/mozilla/activity-stream/commit/36f84f507f54830bf4c5e8db2ad16391f5857571
Fix Bug 1470211 - Dismissing a Bookmarked highlight also dismissed a
Saved To Pocket highlight of the same content - Part 2

https://github.com/mozilla/activity-stream/commit/dfd87e2b4ddf004b00009f92eb814f826c2be549
Merge pull request #4275 from sarracini/bug_1470211

Fix Bug 1470211 - Dismissing a Bookmarked highlight also dismissed a Saved To Pocket highlight of the same content - Part 2

Comment 7

[Ursula Sarracini (:ursula)]

Not resolved just yet, Github robot!

Comment 8

[Ed Lee :Mardak]

Comment on attachment 8996426 [details]
Bug 1470211 - Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1

https://reviewboard.mozilla.org/r/260524/#review267624

We probably want to add to the xpcshell test to make sure this doesn't regress.

::: toolkit/modules/NewTabUtils.jsm:820
(Diff revision 2)
>      // Filter out blocked if necessary
>      if (!aOptions.ignoreBlocked) {
> -      aLinks = aLinks.filter(link => !BlockedLinks.isBlocked(link));
> +      aLinks = aLinks.filter(link => {
> +        // If it is a pocket link, check the open_url field, since that is what we blocked originally
> +        if (link.pocket_id) return !BlockedLinks.isBlocked({url: link.open_url});
> +        return !BlockedLinks.isBlocked(link);

nit: you could…

filter(link => !BlockedLinks.isBlocked(link.pocket_id ? {url: link.open_url} : link));

Comment 9

[Ed Lee :Mardak]

Comment on attachment 8996426 [details]
Bug 1470211 - Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1

https://reviewboard.mozilla.org/r/260526/#review267630

Comment 10

[Ursula Sarracini (:ursula)]

Comment on attachment 8996426 [details]
Bug 1470211 - Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260526/diff/2-3/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/675e65d26f41
Dismissing a Bookmarked highlight also dismisses a Saved To Pocket highlight of the same content - Part 1 r=Mardak

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/675e65d26f41

Comment 13

[Ed Lee :Mardak]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is this something we plan to let ride the trains?

Comment 15

[Ursula Sarracini (:ursula)]

Yes, it will just ride the trains. No need to uplift :)

Comment 16

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 62.0a1 (2018-06-21) with Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20180827100129
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Comment 17

[Patriciu Pop [:ppop], Ecosystem QA]

I have verified that this issue is no longer reproducible on Windows 10 X64, Mac 10.13.3 and Ubuntu 16.04 using the latest "Firefox Nightly" (Version 63.0a1, Build ID 20180903220141).