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

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Robin Whittleton]

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.

Comment 1

[Ian Moody [:Kwan] (UTC+0)]

Just hit this, I think it's fallout from bug 951651.

Comment 2

[Marco Bonardo [:mak]]

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.

Comment 3

[Marco Bonardo [:mak]]

well, not just the yield, linkURI is in the wrong scope and we can simplify the code.

Comment 4

[Marco Bonardo [:mak]]

Attachment: patch v1

Comment 5

[Marco Bonardo [:mak]]

I likely need to unbitrot this against Bug 1135619

Comment 6

[Marco Bonardo [:mak]]

Attachment: patch v2

Unbitrotted patch

Comment 7

[Marco Bonardo [:mak]]

[Tracking Requested - why for this release]: recent regression, broken functionality

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Since this is a recent regression and affects both firefox40 & firefox 41, adding tracking flags for firefox40 and firefox41.

Comment 9

[Pulsebot]

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

Comment 10

[Marco Bonardo [:mak]]

I'm preparing the Aurora version of the patch, will ask for approval on that.

Comment 11

[Marco Bonardo [:mak]]

Attachment: aurora patch v1

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

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/4db392b8bed0

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8608162 [details] [diff] [review]
aurora patch v1

Recent regression, taking it.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f74f78b50255