Closed
Bug 769886
Opened 10 years ago
Closed 10 years ago
Add a "Copy link location" action to the context menu for links
Categories
(Firefox for Android Graveyard :: General, enhancement, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 17
People
(Reporter: mbrubeck, Assigned: mcomella)
References
Details
(Keywords: mobile, platform-parity, polish)
Attachments
(1 file, 2 obsolete files)
1.60 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #643191 +++ We added this to XUL Fennec, and we should add it to Fennec Native for the same reasons.
Reporter | ||
Comment 2•10 years ago
|
||
This is a good bug to get you introduced a bit more to our JavaScript code. The patch from bug 643191 will give you a good idea of how to implement it, though the NativeWindow contextmenu API is slightly different from the old XUL Fennec APIs.
Assignee: mbrubeck → michael.l.comella
Assignee | ||
Comment 3•10 years ago
|
||
I noticed from bug 664798 that "Copy email address", "Copy phone number" and "Copy image location" (if not more) were desired in the previous XUL build and I believe they were lost on the switch to the Native UI. Are these still desired? Should I file separate bugs for them (or do they already exist)? Also, bug 718437 seems like a previously existing meta bug for this issue.
Assignee | ||
Comment 4•10 years ago
|
||
Changed (In reply to Michael Comella (:mcomella) from comment #3) > I noticed from bug 664798 that "Copy email address", "Copy phone number" and > "Copy image location" (if not more) were desired in the previous XUL build > and I believe they were lost on the switch to the Native UI. Are these still > desired? Should I file separate bugs for them (or do they already exist)? I changed bug 718437 into an official meta bug. Comments to this question should move to that bug's page.
Assignee | ||
Comment 5•10 years ago
|
||
I am unsure about a few things: 1) The name of the copy action in the context menu. "Copy Link" (which was in XUL) vs. "Copy Link Location" (which is specified in the bug). I chose the former since it was in production at one point. 2) The regex used in the Copyable context. I did not know which URI schemes we would want access to copy. I chose to omit "tel" and "mailto" as these would be covered by "Copy phone number" and "Copy email address" respectively (which are not currently implemented - see meta bug 718437). I am not sure if anything else should be omitted.
Attachment #643684 -
Flags: review?(mark.finkle)
Comment 6•10 years ago
|
||
Comment on attachment 643684 [details] [diff] [review] Patch >diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js >+ linkCopyableContext: { >+ matches: function linkCopyableContextMatches(aElement) { >+ let uri = NativeWindow.contextmenus._getLink(aElement); >+ if (uri) { >+ let scheme = uri.scheme; >+ let dontCopy = /^(javascript|mailto|tel)$/; Desktop allows javascript: links too, so we can too. Those are used for bookmarklets IIRC. http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js?force=1#359 >diff --git a/mobile/android/locales/en-US/chrome/browser.properties b/mobile/android/locales/en-US/chrome/browser.properties >+contextmenu.copyLink=Copy Link Yeah, "Copy Link" fits in better with our other actions too. r+ with the javascript: change. Post a new patch for check-in with that change. Carry the r+ forward.
Attachment #643684 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #6) > Comment on attachment 643684 [details] [diff] [review] > Patch > > Desktop allows javascript: links too, so we can too. Those are used for > bookmarklets IIRC. Done. Moved r+ forward.
Attachment #643684 -
Attachment is obsolete: true
Attachment #643917 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Keywords: checkin-needed
Reporter | ||
Comment 8•10 years ago
|
||
Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/770e6becb5e4
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/770e6becb5e4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Assignee | ||
Comment 10•10 years ago
|
||
Broken with Bug 766275. Will post patch shortly.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 11•10 years ago
|
||
For breaking it, you get to review it. :P You might want to double check Patch v2 for the other additions I reference, such as the string additions.
Attachment #644515 -
Flags: review?(wjohnston)
Assignee | ||
Updated•10 years ago
|
Attachment #643917 -
Attachment is obsolete: true
Comment 12•10 years ago
|
||
Comment on attachment 644515 [details] [diff] [review] Patch v3 Review of attachment 644515 [details] [diff] [review]: ----------------------------------------------------------------- Lol. Sorry again :(
Attachment #644515 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 13•10 years ago
|
||
No problem. I wish all my reviews were this fast! :)
Keywords: checkin-needed
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbe97ca5ab1c
Flags: in-testsuite-
Keywords: checkin-needed
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fbe97ca5ab1c
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 16•10 years ago
|
||
Build ID: 17.0a2 (2012-10-08)Aurora Channel 18.0b1 (2012-10-08) Nightly Channel 19.0b1 (2012-10-09) Nightly Channel Device: Samsung Galaxy Nexus OS: Android 4.1 Copy link location is now added to the context menu for regular links. Marking bug as verify fixed.
Status: RESOLVED → VERIFIED
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•