Closed Bug 1163576 Opened 5 years ago Closed 5 years ago
Unable to remove pages from the Pocket list
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?
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!
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.
Removed extra ; per review from Michael
Attachment #8604195 - Flags: review?(dolske) → review+
Assignee: nobody → nate
Status: NEW → ASSIGNED
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
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
Nate: Is this no longer an issue for 38.0.5, or for 39? Or for both?
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.
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.
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.)
Attachment #8604195 - Flags: approval-mozilla-release?
Attachment #8604195 - Flags: approval-mozilla-release+
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.
You need to log in before you can comment on or make changes to this bug.