1.33 KB, patch
|Details | Diff | Splinter Review|
338 bytes, text/html
1.14 KB, patch
|Details | Diff | Splinter Review|
5.62 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040506 Firefox/0.8.0+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040506 Firefox/0.8.0+ In addition to the "Copy Link Location" option in the contextual menu for hyperlinks, it would be nice if there were a "Copy Link Text" to copy the contents of the link (because it can be tricky to click and select that text since without following the link itself). Reproducible: Always Steps to Reproduce:
I can't seem to find any open bug for this (or anything all that close)... so... marking new and setting Hardware and OS to all.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: MacOS X → All
Hardware: Macintosh → All
Using Deer Park Alpha 2. Selecting a link text doesn't provide the "copy" option also. It's expected for the user to always have the "copy" option if something is selected. You may textualize it a bit more, you always have to provide these standard options. (Using the Edit > Copy menu does copy the highlighted link text without problems)
Assignee: nobody → ehsan.akhgari
Target Milestone: --- → Firefox 3 M11
Version: unspecified → Trunk
Created attachment 295551 [details] [diff] [review] Patch (v1) This patch adds the "Copy Link Text" context menu item to the context area context menu. This menu item is shown only when clicking on a non-image link when there's no text selected. The reason I made sure to only add this item when no text is selected is having both a "Copy" and a "Copy Link Text" link seems to be confusing, and if the user has selected a part of the document and is looking for a copy command in the context menu, it's likely that she's looking to copy what she's selected, not the text of the link she's clicking on...
(In reply to comment #0) >it can be tricky to click and select that text without following the link itself Hold down Alt while selecting the text.
(In reply to comment #5) > Hold down Alt while selecting the text. Hmm, on OS X that works, but also causes the link to be saved (download manager pops up as it's saved to disk).
Comment on attachment 295551 [details] [diff] [review] Patch (v1) Cancelling ui-review request. I ui-r on security stuff only, I recommend asking mconnor or beltzner for ui reviews the rest of the time.
Created attachment 298774 [details] [diff] [review] Patch (v1) (strings only) (checked in) String changes to land before the l10n freeze...
Attachment #295551 - Flags: ui-review?(mconnor) → ui-review?(beltzner)
Summary: I wish that "Copy link text" was a hyperlink contextual menu option → I wish that "Copy link text" were a hyperlink contextual menu option
Beltzner: can you do a review on the string changes (attachment 298774 [details] [diff] [review]) so that this has a change to land in Firefox 3? Thanks!
Comment on attachment 298774 [details] [diff] [review] Patch (v1) (strings only) (checked in) r+a=beltzner for 1.9 I'm not sure that we actually want to do this, but I'm fine with getting the string in there earlier and thinking more about it. My concern is that it seems like a fairly edge case of functionality (as you'd be copying just the link, not any surrounding text) and "Copy Link Text" is an awkward way to do what essentially is a "copy" operation.
Requesting a check-in for attachment 298774 [details] [diff] [review]...
Whiteboard: [has patch][string changes need to land before l10n freeze]
Attachment #298774 - Attachment description: Patch (v1) (strings only) → Patch (v1) (strings only) (for check-in before the l10n freeze)
Checking in browser/locales/en-US/chrome/browser/browser.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v <-- browser.dtd new revision: 1.93; previous revision: 1.92 done
Whiteboard: [has patch][string changes need to land before l10n freeze] → [has patch]
Attachment #298774 - Attachment description: Patch (v1) (strings only) (for check-in before the l10n freeze) → Patch (v1) (strings only) (checked in)
(In reply to comment #10) > (From update of attachment 298774 [details] [diff] [review]) > r+a=beltzner for 1.9 > > I'm not sure that we actually want to do this, but I'm fine with getting the > string in there earlier and thinking more about it. OK, now that the string is checked in... > My concern is that it seems like a fairly edge case of functionality (as you'd > be copying just the link, not any surrounding text) Actually it might not be an edge case for all users. I've been dependent on an extension which provides this for years. I think it's a logical counterpart to the "Copy Link Location" item. Also, please consider that currently there is no easy way to select a link and copy its contents instead of its location. The operation of selecting a link with the mouse is error-prone (the user may accidentally click the link) and requires careful attention on part of the user. I've seen many users which select the link together with some content around it, copy it, and paste it somewhere else and then edit the pasted text. But this doesn't seem like a user-friendly way of doing things. > and "Copy Link Text" is an > awkward way to do what essentially is a "copy" operation. If the name feels wrong, we can change it to something simpler, like "Copy", but in my opinion we'd lose accuracy in that case, because copying a link and pasting it into an HTML editor should paste the link with its target, and not just the text.
Target Milestone: Firefox 3 beta3 → Firefox 3 beta4
Beltzner: ping for ui-r... Gavin: ping for code review...
Whiteboard: [has patch] → [has patch] [needs ui-review beltzner] [needs review gavin]
Comment on attachment 295551 [details] [diff] [review] Patch (v1) r=me code wise, but I'm not sure this is all that useful.
Attachment #295551 - Flags: review?(gavin.sharp) → review+
Whiteboard: [has patch] [needs ui-review beltzner] [needs review gavin] → [has patch] [needs ui-review beltzner]
Whiteboard: [has patch] [needs ui-review beltzner] → [has patch][needs ui-review beltzner][not needed for 1.9]
Created attachment 327035 [details] testcase Here is a bit hard to copy only the text of the link, i hope gavin is convinced now :) There are other situations when copying the text of the link is not easy as copying text. Padding, float, display and maybe other css stuff are making this harder.
Beltzner, can you look into this?
(I agree this is worth doing.) In reply to comment #6: > > Hold down Alt while selecting the text. > Hmm, on OS X that works, but also causes the link to be saved (download manager > pops up as it's saved to disk). Also on Windows (Firefox 3.0.5). Page http://support.mozilla.com/en-US/kb/Mouse+shortcuts implies this is intentional. Perhaps the additional functionality pointed out in comment #5 is accidental? (But good to know.) In reply to comment #2: > Selecting a link text doesn't provide the "copy" option also. This is no longer true. (Not surprising since that comment is 3.5 years old.)
ColT http://www.borngeek.com/firefox/colt/ solves this.
beltzner: sure, it's edge case, but it's in the land of edge cases - even using a context menu is edge case. How many people do you think really use Properties on a link? But for the people who need it, like me at work where I have to copy linked author names and linked titles all day long, either using one of the three separate extensions which do this (none of which are ever compatible with trunk), or by starting my selection out in whitespace and ending in whitespace so I don't accidentally follow a link, then pasting somewhere that I can see the whitespace to remove it, then pasting where I want it, it's a mighty important/frustrating edge case. And, since making localizers translate strings blind because they're unused for two releases straight isn't very nice behavior, could we maybe get some resolution here after 14 months?
There are at least three extensions that do this: 1. https://addons.mozilla.org/en-US/firefox/addon/1812 - CoLT (Copy Link Text) 2. https://addons.mozilla.org/en-US/firefox/addon/4750 - Copy Link Text 3. https://addons.mozilla.org/en-US/firefox/addon/553 - Copy Link Name I agree that it should be built into Firefox. cf. Bug 378775 - make it easier to select link text
Created attachment 422451 [details] [diff] [review] Backout prelanded strings [checked in] Stealing for a bit with Ehsan's permission, to see if I can push toward some sort of resolution. Step one: whether or not this eventually lands, we need to back out the strings that were prelanded two years ago (and remember things like this any time we're thinking about prelanding strings for anything other than an absolute hard blocker that will without question land), since they have been localized without being used through three releases, and by now the odds are too great that someone decided that they could use the same letter for another accesskey, since they didn't see Copy Link Text showing up for a right click on any of the things that show Foox.
Attachment #422451 - Flags: review?(gavin.sharp) → review+
Created attachment 422455 [details] [diff] [review] Unrotted patch Undoes a little rot, and fixes up the tests that have grown up in the intervening time to test this too. Conveniently, we now publish a nice example of why you might need this: as I do all too often, load http://tests.themasta.com/tinderboxpushlog/ and try to copy one of the linked changeset ids to stick in a "Fixed by backout in 123456aaff" comment, noting how if you start in just the wrong place, you'll have a selection going from right to left while you move the mouse left to right, and once you get a selection, right-click+Copy will generally get you both the selected changeset id and the unselected tooltip for the commit message.
Whiteboard: [has patch][needs ui-review beltzner][not needed for 1.9]
Created attachment 422459 [details] [diff] [review] Really unrotted and tested patch Yeah, let's try that with a qdiff *after* I fix up the test, instead of before.
Comment on attachment 422451 [details] [diff] [review] Backout prelanded strings [checked in] http://hg.mozilla.org/mozilla-central/rev/b91227360383
Attachment #422451 - Attachment description: Backout prelanded strings → Backout prelanded strings [checked in]
Sounds to me like this would almost need to be a reverse parser of bug 515512 or: http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/streamconv/public/mozITXTToHTMLConv.idl
I don't have the slightest idea what that means that you think this bug is about, but, no. All that it needs to do is to put the text that would be used as the name of a bookmark if you used the Bookmark This Link context menuitem into the clipboard, which is exactly what the patch does.
So as it stands, should the strings also get backed out of the 1.9.2 and 1.9.1 branches? Or does that take too much l10n overhead to be worth it?
blocking1.9.2: --- → ?
status1.9.1: --- → ?
Removing an unused string violates string freeze just as much as adding a (used or unused) string would, so no, we won't be doing that.
blocking1.9.2: ? → ---
status1.9.1: ? → ---
(In reply to comment #30) > I don't have the slightest idea what that means that you think this bug is > about, but, no. All that it needs to do is to put the text that would be used > as the name of a bookmark if you used the Bookmark This Link context menuitem > into the clipboard, which is exactly what the patch does. I guess it sounded like from comment 10, comment 13 and comment 26, it would be copy the text of a link (and I was familiar with the reverse process implementation in bug 515512), but your saying that *is* the bookmark name? (If so, then I didn't realize that and comment 29 was in reference to a reverse implementation that exists already) and not copy the link itself. Since I didn't get this UI to come up, I'm not sure of what the actually results are of using the bookmark name here. It sounded like a problem from comment 26 and that maybe it wasn't working quite right or had odd behavior.
(In reply to comment #33) > but your saying that *is* the bookmark name? No, he's saying that it copies the same string that we'd use as the bookmark name if you instead selected "bookmark this link", i.e., the linked text itself. There's no "parsing" involved, and bug 515512's only relation is that it also affects the context menu.
Attachment #422459 - Flags: review?(gavin.sharp) → review+
Comment on attachment 422459 [details] [diff] [review] Really unrotted and tested patch I'm giving this a +, but am honestly not all that sure this is an overall improvment. It's of course true that for users who do this operation regularly, this is a lot easier than trying to select the hyperlink without accidentally clicking on it. However, the significant downside is that this is going to slightly slow down every user who tries to copy link location (they will see both, and need to think about it for half a second before choosing). Overall this does seem to fit the general design of context menus in Firefox, providing a number of different options for each object, but we might in the future want to scale down all of context menu commands based on a quantitative analysis of usage.
Attachment #422459 - Flags: ui-review?(faaborg) → ui-review+
Thanks, faaborg - at least at this point in the cycle, worst case I'll get to use it for a while before someone yanks it :) http://hg.mozilla.org/mozilla-central/rev/15e478e4abd8
Assignee: philringnalda → ehsan.akhgari
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Nice, this works great for copying bug #'s instead of their links to put it somewhere else. I like it also to copy hyperlinked story headlines rather than the link if I want to save an image related to a webpage w/o proper naming! I don't have to cut and paste, use the keyboard to type more than I have to. Thanks.
Verified Fixed on Trunk. Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20100125 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20100125052940
Status: RESOLVED → VERIFIED
Well, this teaches me that I shouldn't ignore my bugmail. I'm sorry I never got around to the ui-r here; I would have pretty clearly said "no, upon sober second thought, this is an edge case only and we shouldn't negatively impact the millions of users who are looking for 'Copy Link' for the benefit of the thousands of users who may be looking to 'Copy Link Text'." I actually suggest that we revert the change and WONTFIX this bug. My fault for dilly dallying here, and not being stronger two years ago when I was first asked to approve the strings. I never should have let that happen. :(
I've backed this out.
Status: VERIFIED → RESOLVED
Last Resolved: 9 years ago → 9 years ago
Resolution: FIXED → WONTFIX
Target Milestone: Firefox 3.7a1 → ---
Dao's backout is http://hg.mozilla.org/mozilla-central/rev/e5210d6b9e7c
Whiteboard: [see comment 24 for extensions which provide this functionality]
You need to log in before you can comment on or make changes to this bug.