Unable to remove pages from the Pocket list

VERIFIED FIXED in Firefox 38.0.5

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: cornel_ionce, Assigned: nate)

Tracking

38 Branch
Firefox 41
Points:
---

Firefox Tracking Flags

(firefox38.0.5 verified, firefox39 fixed, firefox40 verified, firefox41 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
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?
Flags: needinfo?(cornel.ionce)
(Assignee)

Comment 2

4 years ago
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!
(Assignee)

Comment 3

4 years ago
Posted patch FixSavedUrl.patch (obsolete) — Splinter Review
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.
Attachment #8604178 - Flags: review?(michael)
Attachment #8604178 - Flags: review?(dolske)
(Assignee)

Comment 4

4 years ago
Removed extra ; per review from Michael
Attachment #8604178 - Attachment is obsolete: true
Attachment #8604178 - Flags: review?(michael)
Attachment #8604178 - Flags: review?(dolske)
Attachment #8604195 - Flags: review?(michael)
Attachment #8604195 - Flags: review?(dolske)

Updated

4 years ago
Attachment #8604195 - Flags: review?(michael) → review+
Attachment #8604195 - Flags: review?(dolske) → review+
https://hg.mozilla.org/integration/fx-team/rev/e97b29b20465
Assignee: nobody → nate
Status: NEW → ASSIGNED
Flags: needinfo?(cornel.ionce)
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
Attachment #8604195 - Flags: approval-mozilla-release?
Attachment #8604195 - Flags: approval-mozilla-aurora?
Attachment #8604195 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/e97b29b20465
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(Assignee)

Comment 8

4 years ago
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
Flags: qe-verify+
QA Contact: andrei.vaida
Nate: Is this no longer an issue for 38.0.5, or for 39?  Or for both?
Flags: needinfo?(nate)
(Assignee)

Comment 10

4 years ago
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.
Flags: needinfo?(nate)
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.
Flags: needinfo?(nate)
Also, comment 0 says that 40 was not affected. So we may not want to uplift this change to aurora.
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.)
Flags: needinfo?(nate)
Attachment #8604195 - Flags: approval-mozilla-release?
Attachment #8604195 - Flags: approval-mozilla-release+
Attachment #8604195 - Flags: approval-mozilla-beta?
Attachment #8604195 - Flags: approval-mozilla-beta+
Attachment #8604195 - Flags: approval-mozilla-aurora?
Attachment #8604195 - Flags: approval-mozilla-aurora+
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).
Already verified on Firefox 38.0.5, 40, and 41. I don't think it needs additional verification on Firefox 39.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.