Closed Bug 666964 Opened 13 years ago Closed 13 years ago

Prepend http:// to URL copy selection if the first character is included in the selection

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 8
Tracking Status
firefox7 + fixed

People

(Reporter: Margaret, Assigned: Gavin)

References

Details

Attachments

(1 file, 4 obsolete files)

From bug 665580 comment 9: It would be really useful if the copy behaviour matched Chrome too: If the first character is included in the copy selection then prepend the protocol. I've run into this since the patch for bug 665580 landed and agree with Chrome's approach. This only applies to the case where you are copying the beginning of the URL, not the entire URL.
Assignee: nobody → gavin.sharp
Attached patch patch? (obsolete) (deleted) — Splinter Review
This is the approach I took initially because I didn't want to depend on the specifics of trimURL and simply prepend "http://" (particularly when it does not always strip "http://"). However it may be a little bit weird to have the encoded/decoded behavior for partial selections depend on where the selection starts. It may be worth investigating what Chrome does.
Attached patch WIP (obsolete) (deleted) — Splinter Review
forgot to qref
Attachment #541789 - Attachment is obsolete: true
Example of a usability issue solved by the suggested behavior: I'm at <http://example.com/article#comments>. I want to send the article to a friend. I go to the URL, remove "#comments" and then copy it. I now have "example.com/article" on the clipboard. Pasting it into an email or instant messenger usually won't highlight the URL. With this suggestion, the copied URL will include the protocol prefix.
Attached patch patch (obsolete) (deleted) — Splinter Review
That approach didn't work because of encoding differences between the URI and the URL bar value. This patch just explicitly re-adds the trimmed portion when trimmed URLs are selected beginning at the start. I also reworked _getSelectedValueForClipboard a little bit.
Attachment #541790 - Attachment is obsolete: true
Attachment #541976 - Flags: review?(dao)
Comment on attachment 541976 [details] [diff] [review] patch >+ copyVal: "[e]xample.com", I'd prefer <> instead of [], as these are less likely to appear in actual URLs. >+ is(gURLBar.value, test.expectedURL, "url bar value is correct: " + test.expectedURL); No need to spam logs with the expected URLs, is() will report them for failures. I'd suggest "checking urlbar value" or even just "urlbar value", since "TEST-UNEXPECTED-FAIL | foo is correct" is slightly strange. >+ info("Expecting copy of: " + targetValue); Looks like waitForClipboard should just do this if really deemed necessary. >+ is(gBrowser.currentURI.spec, aURL, aURL + " loaded"); see above >--- a/browser/base/content/test/browser_urlbarTrimURLs.js >+++ b/browser/base/content/test/browser_urlbarTrimURLs.js > function testCopy(originalValue, targetValue, cb) { > waitForClipboard(targetValue, function () { >- is(gURLBar.value, originalValue); >+ is(gURLBar.value, originalValue, "URL bar value is " + originalValue); ditto >--- a/browser/base/content/urlbarBindings.xml >+++ b/browser/base/content/urlbarBindings.xml >+ // If the URL bar is modified, nothing else to do here. >+ if (this.getAttribute("pageproxystate") != "valid") >+ return selectedVal; You can return early if selectionStart is > 0 as well. >+ } else if (this.selectionStart == 0 && spec != trimURL(spec)) { This looks like it should call this.trimValue, in case trimming is disabled. >+ // NOTE: this assumes trimURL will can only truncate the URL at "will can" Has "NOTE:" a formalized meaning or is this just there to grab attention? >+ let trimmedSegments = spec.split(trimURL(spec)); Can you put the trimmed value in a variable rather than trimming twice?
(In reply to comment #7) > No need to spam logs with the expected URLs, is() will report them for > failures. I did that intentionally because I actually appreciate the "spam" - it makes it clear in the log which cases are being tested, and makes debugging the test easier (seeing the full progression of checks for a given value rather than generic "test pass" messages). > >+ info("Expecting copy of: " + targetValue); > > Looks like waitForClipboard should just do this if really deemed necessary. It accepts a test function instead of an expected value, so it can't always do that. I had a separate patch that improved output for clipboard failures that I planned to follow up on separately (https://people.mozilla.com/~gavin/clipboardDebug.diff). > Has "NOTE:" a formalized meaning or is this just there to grab attention? The latter.
Attached patch updated patch (obsolete) (deleted) — Splinter Review
Attachment #541976 - Attachment is obsolete: true
Attachment #543032 - Flags: review?(dao)
Attachment #541976 - Flags: review?(dao)
Comment on attachment 543032 [details] [diff] [review] updated patch >+ let trimmedSpec = this.trimValue(spec); > >+ // If the entire URL is selected, just use the actual loaded URI. >+ if (inputVal == selectedVal) { >+ // ... but only if isn't a javascript: or data: URI, since those >+ // are hard to read when encoded >+ if (!uri.schemeIs("javascript") && !uri.schemeIs("data")) { > // Parentheses are known to confuse third-party applications (bug 458565). >- val = val.replace(/[()]/g, function (c) escape(c)); >+ selectedVal = uri.spec.replace(/[()]/g, function (c) escape(c)); > } You could: 1. return selectedVal here, 2. remove the following 'else', 3. then declare trimmedSpec 4. and either check if (spec != trimmedSpec) or return early if (spec == trimmedSpec). >+ } else if (spec != trimmedSpec) { >+ // If the URL is trimmed, we want to prepend the portion that >+ // trimValue removed from the beginning. >+ // This assumes trimValue will only truncate the URL at >+ // the beginning or end (or both). >+ let trimmedSegments = spec.split(trimmedSpec); >+ selectedVal = trimmedSegments[0] + selectedVal; > } > >- return val; >+ return selectedVal;
Attachment #543032 - Flags: review?(dao) → review+
(In reply to comment #8) > (In reply to comment #7) > > No need to spam logs with the expected URLs, is() will report them for > > failures. > > I did that intentionally because I actually appreciate the "spam" - it makes > it clear in the log which cases are being tested, and makes debugging the > test easier (seeing the full progression of checks for a given value rather > than generic "test pass" messages). In that case is() itself should just output the expected value, but I suspect we don't really need/want that. What you usually want to debug is the failure case. I had the pleasure to sift through debug mochitest-other logs. They're currently about 27 MB in size and cause Firefox as well as some text editors to grind to death. They're also unpleasant to look at because of the amount of data you're currently uninterested in.
The test cases show selecting the "e" from example.com prepends the http://. I'd argue that the http:// is only useful if the full domain name is copied -- http://e isn't useful.
(In reply to comment #13) > The test cases show selecting the "e" from example.com prepends the http://. > I'd argue that the http:// is only useful if the full domain name is copied > -- http://e isn't useful. That's probably true, but more complicated to implement. It'd be great if you could file that as a followup bug.
I think that the essence of this bug should be restated so that correct behavior will get implemented. It should be possible to copy a part of a URL without muddying it with "http://"
Depends on: 669483
Attached patch comments addressed (deleted) — Splinter Review
Attachment #543032 - Attachment is obsolete: true
Since nobody seems to have mentioned it yet, I'd like to point out that not only copy operations but also drag and drop operations should be affected by this. A method I often use to load the current URL into another browser (i.e. Safari on Mac) is to drag the URL from the Firefox location bar to the Safari location bar, and this no longer works, since Safari location bar (and supposedly other applications too) seems to be a drop target only for strings that represent URLs. I've tested the same by dragging URLs from the Chrome location bar to Safari, and it correctly adds the protocol to complete the URL.
(In reply to comment #19) > Since nobody seems to have mentioned it yet, I'd like to point out that not > only copy operations but also drag and drop operations should be affected by > this. Can you file a new bug, and CC me?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Attachment #544875 - Flags: approval-mozilla-aurora?
Why not copy the behaviour of Opera? In my opinion it's much better.
Could we get this in Aurora too? Without it there is a change in behavior from the previous versions of Firefox.
(In reply to comment #23) > Could we get this in Aurora too? > > Without it there is a change in behavior from the previous versions of > Firefox. +1. I frequently run into the problem now that I change a URL a little bit, then copy it out of the URL bar, and end up without a protocol. Sadface.
Attachment #544875 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to nightson1988 from comment #22) > Why not copy the behaviour of Opera? In my opinion it's much better. I agree with that.
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0 Verified fixed on Firefox beta using the following steps to reproduce: 1) Access any site (www.google.ro, for example) 2) Copy the link from the location bar. 3) Paste in a text editor The protocol of the website is also displayed when pasting in a text editor. Setting resolution to Verified Fixed.
Status: RESOLVED → VERIFIED
One could object that if you select only the domain name in the url to paste it in a terminal to ping the hostname, you don't want http:// to be added.. oh well :)
Good point, Landry! They should've left things as they were without trying to be clever about the URL. They've opened a can of worms, and some of the worms are still crawling around.
Blocks: 689872
So this is the reason why I have to go to about:config and tell firefox 7 not to hide the http protocol. What a stupid idea to hide it. Undo both "fixes" that break this functionality.
No longer blocks: 689872
Depends on: 691711
No longer depends on: 691711
(In reply to B.J. Herbison from comment #13) > The test cases show selecting the "e" from example.com prepends the http://. > I'd argue that the http:// is only useful if the full domain name is copied > -- http://e isn't useful. FYI: I submitted a bug specifically for this: Bug 707567
I have issues with this behaviour. For e.g. I am veiwing a site and I want to nslookup the domain. I select www.site.com and then attempt to "nslookup [paste]" which fails miserably. HOW do you guys just submit a feature request as a bug report and actually get it resolved? Every time I submit something I just get a "sorry this is the behviour live with it" type of response.
Please don't respond to closed bugs. Use the mozilla support forums. But very few people need to do what you do, so its not a compelling reason to roll it back, IMO. But for you and any others who may find this bug via google, It is a simple fix: * Type "about:config" in the urlbar. * Search for "browser.urlbar.trimURLs" and set it to false. Disliked feature disabled.
I think apple is the only one that did this feature the correct way. They don't show the http:// when showing the url. But if you click on the url to select some of it or change some of it, http:// is put right back in. But either way, this "feature" annoys the hell out of me. I don't want to have to go into about:config every time to disable it. Besides, you show https:// already, why don't you just make it consistent and either show both or show none?
And I would like to point out one thing, every single program out there expects a url to begin with http:// or https:// in order for it to pick up it is a url and make it clickable. It's a standard, don't hide it.
Carl, I am testing with Opera Browser Version 11.64 and it hides http:// and https:// prefix of the URL when focus is brought to the URL bar the prefix is un-hidden. Regardless of the state, the URL does not move in position which is pretty neat. This proposed new bar is even more convoluted: http://cl.ly/401E0Z3A0e1F3T2u1J3C
This bug has been closed and has had this behavior through several releases now. If you have a personal use case, either make an addon, or file bugs blocking this one. In the meantime quit spamming this bug and take the discussion to Google Groups. http://groups.google.com/group/mozilla.dev.apps.firefox/topics?lnk
This bug (or one of it's dupes) should have been kept open in the first place... Only a very small minority agreed on closing it...
Clearly this was not thoroughly thought through.
Kelly: It is not about who wants or doesn't want the http:// perpended. It is the fact that the behaviour of a fundamental operating system feature (copy/cut+paste) have been modified. If you select certain text and then copy it, you do not expect any text but what you copied to end up on the clipboard.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: