1163576 - Unable to remove pages from the Pocket list

Status: VERIFIED FIXED

(Firefox :: Pocket, defect)

Description

[Cornel Ionce [:noni] [Hubs QA]]

Reproducible on:
Firefox 38.0.5b1-build1 (20150510205200)

Affected platforms:
Windows 7 (x64), Windows 8.1 (x86), Mac OS X 10.9.5, Ubuntu 14.04 (x64)

Steps to reproduce:
1. Launch Firefox;
2. Log in to Pocket using a FxAccount.
3. Go to a page (eg http://www.theverge.com/2015/5/10/8581471/parkes-radio-telescope-radio-signals-microwave) and add it to pocket;
4. Select "Remove Page" from the pop-up.
5. Click on "Show Bookmarks" and select view pocket list.


Expected Results:
User is redirected to the pocket list and the deleted page is not shown.

Actual Results:
The deleted page is still displayed on the Pocket list.

Notes:
* Latest Nightly (build ID: 20150510030207) is NOT affected.

Comment 1

[Stuart Philp :sphilp]

I'm unable to reproduce on 38.0.5b1, I can see the page being added/removed from the pocket list. Are there any other specific steps to follow?

Comment 2

[Nate Weiner]

We found the issue. It only happens on if the url that has never been saved to Pocket before (by anyone). So this is also a small case. Expect a patch inbound shortly. Thanks for finding!

Comment 3

[Nate Weiner]

Attachment: FixSavedUrl.patch

Two issues addressed here:
1. Only the given_url should be used for communicating with the server (not resolved)
2. The resolved_url was returning null because it hadn't been parsed before, therefore savedUrl was null and it was falling back to the window.location which inside of a panel is always going to be the chrome address, so that was removed as well.

Comment 4

[Nate Weiner]

Attachment: FixSavedUrlv2.patch

Removed extra ; per review from Michael

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/e97b29b20465

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8604195 [details] [diff] [review]
FixSavedUrlv2.patch

Approval Request Comment
[Feature/regressing bug #]: part of Pocket for desktop
[User impact if declined]: pages that were only added to the Pocket server by one user fail to get deleted
[Describe test coverage new/current, TreeHerder]: manual testing by the Pocket engineers, just landed on fx-team
[Risks and why]: none expected, risks limited to Pocket functionality
[String/UUID change made/needed]: none

Comment 7

[Carsten Book [:Tomcat]]

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

Comment 8

[Nate Weiner]

FYI: This fix should still go out, but wanted to update that we found a way to hack the response server side to account for this, so this is no longer an issue in the current Beta.

Referenced here: https://github.com/Pocket/Firefox/issues/74

Comment 9

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Nate: Is this no longer an issue for 38.0.5, or for 39?  Or for both?

Comment 10

[Nate Weiner]

Liz: This is an issue until the patch in this bug lands. However, in the meantime, we were able to put a temporary hack in place server side to make sure that users do not experience this issue. That said, we should still land the patch so we can remove the temp hack.

Comment 11

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

OK, great. Because of our upcoming extra release, some people have been saying "beta" and meaning either 39, 38.0, or 38.0.5, so I would like to clarify which versions are affected. Looks likely to be all three.

Comment 12

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Also, comment 0 says that 40 was not affected. So we may not want to uplift this change to aurora.

Comment 13

[Justin Dolske [:Dolske]]

Comment on attachment 8604195 [details] [diff] [review]
FixSavedUrlv2.patch

a+ for aurora/beta/release: required for Pocket launch in 38.0.5.

(We very much do want to uplift this and all the other Pocket work, so that what's landing is consistent across branches. Important for testing and sanity of all the other code uplifting.)

Comment 14

[Justin Dolske [:Dolske]]

(aurora currently closed)
https://hg.mozilla.org/releases/mozilla-beta/rev/8b07526d0c92
https://hg.mozilla.org/releases/mozilla-release/rev/f7f9fc975cdc

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/84a249d662b5

Comment 16

[Cornel Ionce [:noni] [Hubs QA]]

Confirming the fix on Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 12.04 32-bit using:
- latest Nightly (build ID: 20150517030204);
- latest Aurora (build ID: 20150517004004);
- Firefox 30.0.5 beta 2 (build ID: 20150514163436).

Comment 17

[Florin Mezei, QA (:FlorinMezei)]

Already verified on Firefox 38.0.5, 40, and 41. I don't think it needs additional verification on Firefox 39.