Closed Bug 1160476 Opened 9 years ago Closed 9 years ago

‘Bookmark this link’ context menu option doesn’t work

Categories

(Firefox :: Bookmarks & History, defect)

40 Branch
defect
Not set
normal
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.1 - May 25
Tracking Status
firefox39 --- unaffected
firefox40 + verified
firefox41 + verified

People

(Reporter: robin, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150430030201

Steps to reproduce:

Right clicked on a link, chose ‘Bookmark This Link’


Actual results:

Context menu disappeared, nothing else. This error appears in the browser console:

A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Fri May 01 2015 13:07:15 GMT+0100 (BST)
Full Message: ReferenceError: linkURI is not defined
Full Stack: PlacesCommandHook.bookmarkLink<@chrome://browser/content/browser.js:5358:42
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
CM_bookmarkLink@chrome://browser/content/nsContextMenu.js:1558:1
oncommand@chrome://browser/content/browser.xul:1:1


Expected results:

A bookmark to that link would appear in the Bookmarks / Recently Bookmarked menu.
Component: Untriaged → Bookmarks & History
Just hit this, I think it's fallout from bug 951651.
Blocks: 951651
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mano)
Flags: needinfo?(mak77)
Keywords: regression
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Flags: needinfo?(mak77)
Points: --- → 2
Flags: qe-verify+
Flags: needinfo?(mano)
Flags: firefox-backlog+
this is just missing a yield... I actually had this fixed already in bug 1094818, but I'll split the fix here to allow for a quicker review.
Points: 2 → 1
well, not just the yield, linkURI is in the wrong scope and we can simplify the code.
Iteration: --- → 40.3 - 11 May
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #8603566 - Flags: review?(ttaubert)
I likely need to unbitrot this against Bug 1135619
Attached patch patch v2Splinter Review
Unbitrotted patch
Attachment #8603566 - Attachment is obsolete: true
Attachment #8603566 - Flags: review?(ttaubert)
Attachment #8605306 - Flags: review?(ttaubert)
[Tracking Requested - why for this release]: recent regression, broken functionality
Since this is a recent regression and affects both firefox40 & firefox 41, adding tracking flags for firefox40 and firefox41.
Iteration: 40.3 - 11 May → 41.1 - May 25
Attachment #8605306 - Flags: review?(ttaubert) → review+
I'm preparing the Aurora version of the patch, will ask for approval on that.
Attached patch aurora patch v1Splinter Review
Approval Request Comment
[Feature/regressing bug #]: bug 951651
[User impact if declined]: Bookmark this link contextual menu doesn't work
[Describe test coverage new/current, TreeHerder]: Nightly, manual testing
[Risks and why]: low risk since reusing existing code path
[String/UUID change made/needed]: none
Attachment #8608162 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4db392b8bed0
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8608162 [details] [diff] [review]
aurora patch v1

Recent regression, taking it.
Attachment #8608162 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.