Closed Bug 1389721 Opened 4 years ago Closed 3 years ago
Right-click > Save Link to Pocket doesn't work
Bug 1389721 - fix page action menu to deal with not being passed an event, so the 'save link to pocket' context menu works,
59 bytes, text/x-review-board-request
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20170811100330 Steps to reproduce: – Right-click a link on a webpage – Select "Save Link to Pocket" in context menu Actual results: There is no indication that the action has been completed, and upon checking the Pocket list, the link is not present. Expected results: The link should be saved to the user's Pocket list.
the issue is reproducible and running mozregression on it points to bug 1355922 as the source of the regression. the browser console logs the following while attempting to save a link to pocket through the context menu: TypeError: event is undefined browser-pageActions.js:464:1 doCommandForAction chrome://browser/content/browser-pageActions.js:464:1 doCommand resource:///modules/PageActions.jsm:705:5 savePage chrome://pocket/content/Pocket.jsm:106:7 oncommand chrome://browser/content/browser.xul:1:1
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Priority: -- → P1
Photon is disabled and Philipp confirmed this doesn't actually reproduce on release, so updating some tracking info. :-)
Comment on attachment 8914479 [details] Bug 1389721 - fix page action menu to deal with not being passed an event, so the 'save link to pocket' context menu works, https://reviewboard.mozilla.org/r/185808/#review190900 Thanks for fixing this and sorry for breaking it :( ::: commit-message-97efd:1 (Diff revision 1) > +Bug 1389721 - fix 'save link as' pocket context menu functionality, r?jaws Please change the commit message to be more descriptive.
Attachment #8914479 - Flags: review?(jaws) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/63c7cfadae7b fix page action menu to deal with not being passed an event, so the 'save link to pocket' context menu works, r=jaws
Comment on attachment 8914479 [details] Bug 1389721 - fix page action menu to deal with not being passed an event, so the 'save link to pocket' context menu works, Approval Request Comment [Feature/Bug causing the regression]: bug 1355922 and then some other photon-related changes [User impact if declined]: completely broken context menu item (save page/link to pocket) [Is this code covered by automated tests?]: no. pocket code doesn't have enough automated tests, unfortunately. I'll file a follow-up for this. We shouldn't be able to break things this badly without the tests yelling at us. [Has the fix been verified in Nightly?]: not yet. [Needs manual test from QE? If yes, steps to reproduce]: see comment #0 [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: well, it's hard to get more broken than completely broken. I guess crashing the browser... In all seriousness, this is a straightforward small nullcheck in the right place, plus removing some dead code. Happily, I'm not trying to crashland this into rc either, but even then I'd be pretty OK with saying we should take this (!). [String changes made/needed]: none
Attachment #8914479 - Flags: approval-mozilla-beta?
I filed bug 1405477 for adding more automated tests for pocket functionality.
Comment on attachment 8914479 [details] Bug 1389721 - fix page action menu to deal with not being passed an event, so the 'save link to pocket' context menu works, We probably want that to be fixed for 57. Taking it to 57b
Attachment #8914479 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have reproduced the issue mentioned in comment 0, using an affected Firefox 57.0a1 build (BuildId:20170811100330). I have verified that the issue is not reproducible using Firefox 57.0b6 (Build Id:20171005195903) and Firefox 58.0a1 (BuildId:20171009100134) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
You need to log in before you can comment on or make changes to this bug.