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)
Firefox
Address Bar
Tracking
()
VERIFIED
FIXED
Firefox 8
People
(Reporter: Margaret, Assigned: Gavin)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
asa
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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 | ||
Updated•13 years ago
|
Assignee: nobody → gavin.sharp
Assignee | ||
Comment 1•13 years ago
|
||
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.
Comment 3•13 years ago
|
||
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.
Assignee | ||
Comment 6•13 years ago
|
||
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
tracking-firefox7:
--- → +
Assignee | ||
Updated•13 years ago
|
Attachment #541976 -
Flags: review?(dao)
Comment 7•13 years ago
|
||
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?
Assignee | ||
Comment 8•13 years ago
|
||
(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.
Assignee | ||
Comment 9•13 years ago
|
||
Attachment #541976 -
Attachment is obsolete: true
Attachment #543032 -
Flags: review?(dao)
Attachment #541976 -
Flags: review?(dao)
Comment 11•13 years ago
|
||
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+
Comment 12•13 years ago
|
||
(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.
Comment 13•13 years ago
|
||
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.
Assignee | ||
Comment 14•13 years ago
|
||
(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.
Comment 15•13 years ago
|
||
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://"
Assignee | ||
Comment 16•13 years ago
|
||
Attachment #543032 -
Attachment is obsolete: true
Assignee | ||
Comment 17•13 years ago
|
||
Status: NEW → ASSIGNED
Comment 19•13 years ago
|
||
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.
Assignee | ||
Comment 20•13 years ago
|
||
(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?
Assignee | ||
Comment 21•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Assignee | ||
Updated•13 years ago
|
Attachment #544875 -
Flags: approval-mozilla-aurora?
Comment 22•13 years ago
|
||
Why not copy the behaviour of Opera? In my opinion it's much better.
Comment 23•13 years ago
|
||
Could we get this in Aurora too?
Without it there is a change in behavior from the previous versions of Firefox.
Comment 24•13 years ago
|
||
(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.
Updated•13 years ago
|
Attachment #544875 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 25•13 years ago
|
||
status-firefox7:
--- → fixed
Comment 27•13 years ago
|
||
(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.
Comment 28•13 years ago
|
||
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
Comment 29•13 years ago
|
||
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 :)
Comment 30•13 years ago
|
||
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.
Comment 31•13 years ago
|
||
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.
Comment 32•13 years ago
|
||
(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
Comment 33•12 years ago
|
||
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.
Comment 34•12 years ago
|
||
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.
Comment 35•12 years ago
|
||
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?
Comment 36•12 years ago
|
||
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.
Comment 37•12 years ago
|
||
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
Comment 38•12 years ago
|
||
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
Comment 39•12 years ago
|
||
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...
Comment 40•12 years ago
|
||
Clearly this was not thoroughly thought through.
Comment 41•12 years ago
|
||
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.
Description
•