Build Identifier: http://hg.mozilla.org/mozilla-central/rev/456c915b3caf Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110524 Firefox/6.0a1 ID:20110524030609 Trailing // should disappear if input text is something like "filxxxx" in the LocationBar Reproducible: Always Steps to Reproduce: 1. Start Firefox/6.0a1 2. Open Local file (Ex. file:///D:/localfile.html ) 3. Key in LocationBar fill Actual Results: fill// Expected Results: fill

Yeah, this is totally broken. Breaks a number of my bookmark keywords, not to mention opening of actual file:// URIs!

(BTW, I don't hit this if I type super-fast. I definitely hit it when typing at the rate of ~2 chars per second, though.)

Taking the issue back one step here -- it looks like with "file:///" completions, the suggested portion isn't fully highlighted. The highlighting stops before the final "//" of the suggestion. So it looks like: file:/// (where 'fi' is what I typed and '^'=highlight) ^^^^ In all other cases I've tested, the *entire* suggested portion is highlighted (and hence deleted when I type another character or hit backspace).

This is caused by not having file:// as one of the valid schemes. Removing http:// from the function causes that to show this behaviour too, strangely...

Wait. Why do we have some sort of string scheme whitelist/blacklist here? Won't this break for other schemes not in this list too?

(In reply to comment #5) > Wait. Why do we have some sort of string scheme whitelist/blacklist here? > Won't this break for other schemes not in this list too? Yes, I didn't realize this would be the interaction with other protocols when I reviewed this initially. Need to come up with a better solution here.

Comment on attachment 535001 [details] [diff] [review] Patch Whitelisting is a losing strategy here. What about other protocols like chrome:? Can you please explain how we end up adding the // in there in the first place (I clearly don't understand this code as well as I thought I did)?

I was doing what Places already does in other parts in JS code. I'm not sure what other strategy we could use. I'm also not sure why the // is added without the selection, since debugging that code shows CompleteValue getting the expected value and range (up to the first /). Considering I may need to take a new approach to fix bug 659445, probably backing out the whole thing seems like a good option. (We can't just turn off the pref as I think I mistakenly removed some important code, see bug 659672)

don't need to track this for 6 since the feature has been removed for 6.