(similar to bug 666964) STR: 1. Type "bug" in your location bar 2. Arrow down to a URL you like the look of 3. Select All, Copy 4. Open a text editor and Paste Expected: http://url-you-liked Actual: url-you-liked

This looks like being fixed on the latest build (win7) I copy a URL from the address bar, http:// is added in the paste result I type a word in the URL bar (which would lead to a dead link), no http:// is added I select only part of a URL in the address bar, no http:// is added This is correct behavior, only the moment you select the full URL in the address bar would be better if it was also visible that you are actually selecting the 'http://'-prefix too. (So that What We See Is What We Get)

(In reply to comment #4) > This looks like being fixed on the latest build (win7) > > I copy a URL from the address bar, http:// is added in the paste result > I type a word in the URL bar (which would lead to a dead link), no http:// > is added > I select only part of a URL in the address bar, no http:// is added > > This is correct behavior, only the moment you select the full URL in the > address bar would be better if it was also visible that you are actually > selecting the 'http://'-prefix too. (So that What We See Is What We Get) Oh, not completely fixed, when I select part of the address bar starting from the beginning of the URL, the http:// is not added in the pasted result.

(In reply to comment #5) > Oh, not completely fixed, when I select part of the address bar starting > from the beginning of the URL, the http:// is not added in the pasted result. That's bug 666964, and I landed its fix on mozilla-central earlier today.

Nothing in comment 4 has anything to do with this bug. For this bug, the "from location bar" part is key. That is, the url was selected from the autocomplete dropdown.

We're into aurora land, but this is bustage that's novel for Aurora 7 - margaret/gavin - is a fix here gonna be safe?

On closer look this is indeed another bug... Why was Bug 670503 marked as duplicate of this one if it's a different bug??

> Why was Bug 670503 marked as duplicate of this one if it's a different bug?? Bug 670503 comment 2 says why...

The easiest way to solve this seems to be to always fixup the selected text. This has the side-effect of making a URL out of anything you enter into the URL bar, but I actually prefer that. It's a bit weird for copy and paste behaviour to depend on whether a page is loaded, after all. The alternative is to set some sort of flag saying that the URL in the text box came from the location bar dropdown, but I have no idea even where to start with that.

This makes me thinking of buying a car with buggy electronics, where electronics aren't actually needed... When a gadget causes more problems and annoyance, than any good, it's better to leave it off altogether, if you want to achieve a solid quality product. A Visual gadget should Never influence a good and clear workflow. I would make the http:// optional & standard off, so it only is applied for those wishing to hide that part of an URL, with the associated annoyances. Standard should always be the best working method. Also it has been proven that for people the What-You-See-Is-What-You-Get always works best. Stepping away from this doesn't seem to be a good idea. Makes things more vague instead of clear...

> This has the side-effect of making a URL out of anything you enter into the URL > bar This fails the "type some stuff, realize you actually wanted it in the search field, copy and paste it". Actual user-typed values should not, imo, be messed with. What would make sense to me is if selecting text from the dropdown actually showed the url until the user presses enter or something.

Comment on attachment 549614 [details] [diff] [review] WIP: fixup URL of selected text Indeed, this is unsuitable because of the problem Boris points out. I had a patch for this that I was playing with that kept the untrimmed value around for use in _getSelectedValueForClipboard, but it was kind of messy, particularly given that we trim the values before they ever get added to the popup. I think we will need something along those lines to properly fix this, though.

Note that this doesn't cover: autocomplete in tab A -> select tab B -> select tab A -> copy We'd need to keep track of more things to fix this.

the previous patch contained changes that aren't needed for the approach I ended up taking

(In reply to Boris Zbarsky (:bz) from comment #16) > > This has the side-effect of making a URL out of anything you enter into the URL > > bar > > This fails the "type some stuff, realize you actually wanted it in the > search field, copy and paste it". > > Actual user-typed values should not, imo, be messed with. Big +1

Is this behavior ok to ship with? Should we back out the feature?

It's a regression of pretty core functionality (copying URLs for sharing elsewhere) but I'd hate to see this single issue resulting in the backout of the entire feature. Can someone pin Gavin down and tempt him with a cookie for the review? Bonus points for getting him beside Dao so any issues can be worked through ...

FWIW, Chrome has this same issue IIRC.

Yes, Chrome has the same issue. Chrome also ships with bug 666964, if that helps drivers decide (fwiw, I don't think that should make it OK for *us* to ship with either of those bugs)

The feature is of dubious value (it hides just 7 characters), but the regression is very inconvenient. I run into it frequently. It doesn't matter what another browser does.

(In reply to Christian Legnitto [:LegNeato] from comment #21) I was arguing for backing this out based on this because we are hearing complaints that users are having trouble switching to https because they're used to doing click on url bar, add "s", enter. It's just become less clear if they're on https or not.

Comment 26 should be filed as it's own bug, blocking bug 665580, and nominated for tracking against the next release of Firefox - it's not really related to the issue in this bug. (I also don't think I agree with it, but we can talk about that in its own bug once filed!)

Comment on attachment 550654 [details] [diff] [review] patch >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js > gURLBar.value = value; >+ gURLBar.valueIsTyped = !valid; Do we really need this change? The only behavior it affects is the blank tab/location bar case, I guess? >diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml > <method name="_getSelectedValueForClipboard"> >+ let uri = uriFixup.createFixupURI(inputVal, Ci.nsIURIFixup.FIXUP_FLAG_NONE); I think it's probably best to wrap this in a try/catch - this can throw for obviously invalid URIs, and in some other cases too (e.g. when passed an empty string). We should add a comment to trimURL mentioning the fact that we rely on it not modifying the value such that a createFixupURI will produce a different URI. We should get test coverage for this.

(In reply to Gavin Sharp from comment #28) > >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js > > > gURLBar.value = value; > >+ gURLBar.valueIsTyped = !valid; > > Do we really need this change? The only behavior it affects is the blank > tab/location bar case, I guess? Without it, valueIsTyped would inconsistently be false when autocompleting in one tab and switching to a tab with pageproxystate = valid (as opposed to typing and loading a page in one tab). > > <method name="_getSelectedValueForClipboard"> > > >+ let uri = uriFixup.createFixupURI(inputVal, Ci.nsIURIFixup.FIXUP_FLAG_NONE); > > I think it's probably best to wrap this in a try/catch - this can throw for > obviously invalid URIs, and in some other cases too (e.g. when passed an > empty string). This shouldn't happen, i.e. we shouldn't have such URIs with valueIsTyped = false. I'll add the try/catch, though. > We should get test coverage for this. Are there existing tests performing on the actual autocomplete UI rather than penetrating parts of the API?

(In reply to Dão Gottwald [:dao] from comment #29) > > >+ let uri = uriFixup.createFixupURI(inputVal, Ci.nsIURIFixup.FIXUP_FLAG_NONE); > > > > I think it's probably best to wrap this in a try/catch - this can throw for > > obviously invalid URIs, and in some other cases too (e.g. when passed an > > empty string). > > This shouldn't happen, i.e. we shouldn't have such URIs with valueIsTyped = > false. I'll add the try/catch, though. Well I guess we can actually get invalid URIs when selecting part of the value as opposed to the whole value.

with test fixes

The only behavior change in this patch is related to copying text from the location bar. I'm still uncomfortable taking this on beta at the last minute, though, since it has the potential to break that functionality (which is fairly important). I think we should take it only on Aurora, and WONTFIX this bug for the Firefox 7 cycle.

I agree with Gavin. We should get this into Aurora.

qa+ for verification on Firefox 8

Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0 Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111003 Firefox/9.0a2 Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111002 Firefox/10.0a1 Verified in F8 Beta 1,Firefox 9 and 10 on ubuntu 11.04, Mac OS 10.6, Windows XP and Windows 7 using the steps in Comment 0.

This does not work in 11.0a1 (2011-11-16)

(In reply to Roman R. from comment #40) > This does not work in 11.0a1 (2011-11-16) Please check if this happens on a new profile (http://kb.mozillazine.org/Profile_manager) as this works fine for me using the same build. It's possible an add-on you have installed is altering this behaviour. If you still can't get this to work, please file a new bug.