contextMenus.OnClickData should have the linkText of the link

RESOLVED FIXED in Firefox 56

Status

()

Toolkit
WebExtensions: General
P5
normal
RESOLVED FIXED
6 months ago
2 days ago

People

(Reporter: kernp25, Assigned: Thomas Wisniewski)

Tracking

({dev-doc-needed})

unspecified
mozilla56
dev-doc-needed
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [design-decision-approved][contextMenus] triaged)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 months ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170227030203



Actual results:

No linkText in contextMenus.OnClickData


Expected results:

contextMenus.OnClickData should have the linkText of the link

Updated

6 months ago
Whiteboard: [design-decision-needed]
I also face to this problem when I port my extension to WebExtensions.

In XUL add-on era, there is `gContextMenu.linkTextStr` to get the link text. But it's not in WebExt API.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

3 months ago
This popular add-on also uses `gContextMenu.linkTextStr`:
https://addons.mozilla.org/firefox/addon/colt/
Hi all, this has been added to the agenda for the May 16 WebExtensions API triage. Kernp25 and Tetsuharu, would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage

Agenda: https://docs.google.com/document/d/1vrhHNOelBty4hXcjQ8VbFk-azHRFFDVyGka7H0VpEa8/edit#
(Reporter)

Comment 4

3 months ago
(In reply to Caitlin Neiman (http://pronoun.is/she) from comment #3)
> Hi all, this has been added to the agenda for the May 16 WebExtensions API
> triage. Kernp25 and Tetsuharu, would you be able to join us? 

I don't think i'm able to join, sorry :(

But here are my thoughts about it:

If the browser already provides the linkText of the link, why not add it to contextMenus.OnClickData?
I don't think this is much trouble.

Because, otherwise we have to call these functions our self again (in content-script):
1) getLinkText [1]
2) gatherTextUnder [2]

We also must do this (to get the link that was clicked):
> while (srcElement && getLinkURL(srcElement) != request.linkUrl) {
>   srcElement = srcElement.parentElement;
> }

I copied the getLinkURL [3] function and modified it a bit:
> function getLinkURL(node) {
>   var href = node.href;
>   if (href)
>     return href;
> 
>   href = node.getAttribute("href") ||
>          node.getAttributeNS("http://www.w3.org/1999/xlink", "href");
> 
>   if (!href || !/\S/.test(href)) {
>     return "";
>   }
> 
>   var url = new URL(href, node.baseURI);
>   return url.toString();
> }

I'm also thinking for performance reasons this is really bad.

[1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#1711
[2] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/utilityOverlay.js#634
[3] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#1676
(Reporter)

Comment 5

3 months ago
I think this is also useful for some translate add-ons to translate the linkText of the link.
Flags: needinfo?(lgreco)

Comment 6

3 months ago
We discussed about this during the last community API triage meeting and we agreed to approve this feature request.

The rationale is that, even if the Bug 1325814 is going to provide the CSS selector of the clicked element and an extension would be able to use it to retrieve the needed webpage content from its own content script, we already have the requested content available in the internals and the use case is common enough to prefer to not force an extension to inject a content script just to retrieve it.
Flags: needinfo?(lgreco)
Priority: -- → P5
Whiteboard: [design-decision-needed] → [design-decision-approved][contextMenus] triaged
(Assignee)

Comment 7

2 months ago
Created attachment 8879381 [details] [diff] [review]
1343236-add_support_for_linkText_to_contextMenus.OnClickData.diff

Will this patch do? It works as expected for me with a simple test addon, but I don't see any existing tests for related properties like linkUrl, and I'm not sure if that's an oversight or not. I can at least do a try run if we'd like?
Attachment #8879381 - Flags: review?(mixedpuppy)
(In reply to Thomas Wisniewski from comment #7)
> Will this patch do? It works as expected for me with a simple test addon,
> but I don't see any existing tests for related properties like linkUrl, and
> I'm not sure if that's an oversight or not. I can at least do a try run if
> we'd like?

Yes, that's an oversight, you should add a test to [1].  Also, you need to add the property to the JSON schema [2].

1) http://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_contextMenus.js
2) http://searchfox.org/mozilla-central/source/browser/components/extensions/schemas/menus.json#75
(Assignee)

Comment 9

2 months ago
Created attachment 8879770 [details] [diff] [review]
1343236-add_support_for_linkText_to_contextMenus.OnClickData.diff

Thanks :zombie! Here's a fresh patch with those changes, try seems fine with it: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bc079c89220084f165153ffd98fbebda4dd9032
Assignee: nobody → wisniewskit
Attachment #8879381 - Attachment is obsolete: true
Attachment #8879381 - Flags: review?(mixedpuppy)
Attachment #8879770 - Flags: review?(mixedpuppy)
Comment on attachment 8879770 [details] [diff] [review]
1343236-add_support_for_linkText_to_contextMenus.OnClickData.diff

Thanks!
Attachment #8879770 - Flags: review?(mixedpuppy) → review+
Keywords: checkin-needed

Comment 11

2 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3bc9a8ef370
Add support for linkText to contextMenus.OnClickData. r=mixedpuppy
Keywords: checkin-needed

Comment 12

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f3bc9a8ef370
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.