If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

trailing // should disappear if input text is something like "filxxxx" in the LocationBar

RESOLVED WORKSFORME

Status

()

Firefox
Address Bar
RESOLVED WORKSFORME
6 years ago
2 years ago

People

(Reporter: Alice0775 White, Assigned: Michael Ventnor)

Tracking

(Blocks: 1 bug, {dogfood, regression})

Trunk
dogfood, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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!
tracking-firefox6: --- → ?
(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.)
tracking-firefox6: ? → ---
Keywords: dogfood, regression
Hardware: x86 → All
tracking-firefox6: --- → ?
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).
Blocks: 659437
(Assignee)

Comment 4

6 years ago
Created attachment 535001 [details] [diff] [review]
Patch

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...
Assignee: nobody → ventnor.bugzilla
Attachment #535001 - Flags: review?(sdwilsh)
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)?
Attachment #535001 - Flags: review?(sdwilsh) → review-
(Assignee)

Comment 8

6 years ago
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)

Updated

6 years ago
Duplicate of this bug: 659848

Comment 10

6 years ago
don't need to track this for 6 since the feature has been removed for 6.
tracking-firefox6: ? → ---

Updated

6 years ago
No longer blocks: 566489
Depends on: 566489

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.