Closed Bug 202992 Opened 21 years ago Closed 18 years ago

"h" or "htt" cut off in address bar autocomplete (adds :// to domain names starting with "p" or "t")

Categories

(Toolkit :: Autocomplete, defect, P4)

Product:
Toolkit
Component:
Autocomplete
Version:
1.8 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8.1

People

(Reporter: mark11, Assigned: pkasting)

References

Details

(Keywords: fixed1.8.1)

Attachments

(1 file, 1 obsolete file)

First cut at fix for browser.urlbar.autoFill == true users.
6.83 KB, patch
brettw
: first-review+
bryner
: second-review+
bryner
: approval-branch-1.8.1+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030422 Firebird™ Browser/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030422 Firebird™ Browser/0.6

Autocomplete address bar entries have the htp at the front cut off when you
attempt to change them with the left and right arrow keys.

Reproducible: Always

Steps to Reproduce:
1. Start typing an address for a site you've visited before to get an auto
complete entry
2. Use the up and down arrows to select an entry
3. Use the left and right arrows to select part of it to change
4. Press enter

Actual Results:  
The "htp" is cut off, and the error message "p is not a registered protocol"
results.

Expected Results:  
Not cut off the "htp", and instead went to the (now edited) URL from autocomplete

I'm unsure, but this may be in some way related to 172530? (though it's not the
same as far as I can tell. Theme doesn't seem to matter.
Appears to be fixed in the latest nightly Mozilla/5.0 (Windows; U; Windows NT
5.1; en-US; rv:1.4a) Gecko/20030424 Mozilla Firebird/0.6 . Now it just cuts off
the entire http:// ; which makes it work right.
Okay, nevermind. I shouldn't have said anything. It was fixed when I first
installed the latest nightly (and I checked several times to be sure, restarting
the browser even). Then I posted here it seemed to be fixed, and it immediately
broke again. So it's not 100% reproducable.
Problem discovered quickly. It only occurs when the first letter of what you
type is p. I hadn't noticed, because I'd visited quite a few sites starting with
p and had never even tried something else.
confirming
-> Severity: normal

Steps to Reproduce:
1. Open (or just click) http://prefbar.mozdev.org/
2. Press "p" on location bar.
3. Press down arrow key (several times) to select "http://prefbar.mozdev.org/"
4. Press left or right arrow key and press enter.

Actual Results:  
The error message "p is not a registered protocol"

Expected Results:  
"htt" should not be cut off.
Severity: major → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: "htp" cut off in address bar autocomplete → "htt" cut off in address bar autocomplete
Taking QA. Sorry for the bugspam
QA Contact: asa → davidpjames

*** This bug has been marked as a duplicate of 197990 ***
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → DUPLICATE
Oops, this bug is for Firebird and that one is for seamonkey.  It's the same
problem, but I think this code has forked, so reopening.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: "htt" cut off in address bar autocomplete → "h" or "htt" cut off in address bar autocomplete
These bugs seem related:
bug 194574, "preview of autocomplete selection not right (capital/lowercase
letters)"
bug 214770, Z autocompletes to Zzap instead of zap.
Upon further investigation, this bug is different than bug 191574 and bug 214770
as this bug is letter-specific. If you follow the instructions in comment #5,
except using 't', you get the same results. 'h' is probably the same as well,
but since it is replacing itself, you don't notice. Those other two have a
different cause.
This bug isn't really letter-specific.  If I type 'u' and press down in the
Firebird address bar, everything up to the 'u' (e.g. "http://www.") is cut off.
That's true, but that doesn't usually prevent a successful loading of the site.
We don't get mp://www.mozilla.org, for example when using m. What seems to be
happening is that autocomplete matches against anything following http://www.
but it cuts out everything before the first typed letter when selecting. So for
anything other than t, p and h that's not really a problem, and even for h it's
not really either. I guess what it should do is not cut off anything at all and
just go with the what's there before you arrow down to it.
I did, however, think of a situation when this bug could potentially affect
other sites. If the site doesn't have the server setup to recognise
site.whatever instead of www.site.whatever . 
I see this too in latest nightlies. To clarify, for me this behavior (cutting of
first couple of letters) occurs only when I try to type characters in different
lettercase than one stored in history. 

For example, if the history contains an entry 'http://mozilla.org' then when
typing 'Moz' and pressing down key for selecting the entry leaves the address
bar in a broken state: 'Moztp://mozilla.org/'. And eventually an attempt to,
say, add 'projects/' to the address results in a error opening
'Moztp://mozilla.org/projects/'

But when typing 'moz' the address bar correctly displays 'mozilla.org/' and lets
one to edit and successfully open a page.
Example @ 13: confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.5b) Gecko/20030823 Mozilla Firebird/0.6.1+, but works for me with
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5b) Gecko/20030824
I'm still seeing this in both build from August, 24 and for example in today's
one (Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030830 Firebird/0.6.1+)
Suddenly it works for me in 

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040318
Firefox/0.8.0+

What about others?
Flags: blocking1.0?
Worksforme too
Status: REOPENED → RESOLVED
Closed: 21 years ago → 20 years ago
Resolution: --- → WORKSFORME
making block incase it gets reopened
Flags: blocking1.0? → blocking1.0+
It's still broken for me. Hmmm.. I dunno what else to say?
*** Bug 242747 has been marked as a duplicate of this bug. ***
This is apparently still causing problems (see duped bug 202992).

If I start typing a url with 'h', 't' or 'p', then arrow down it cuts off at
that character in 'http' in the url bar (but it loads the correct url
nonetheless). In the case of 'h' it cuts off prior to the 'h' so we don't notice
it but it is different than typing 'm' for mozilla.

1. Visit http://www.hotbot.com/
2. focus url bar, type 'h', arrow down to hotbot entry
3. h<highlight>ttp://www.hotbot.com/</highlight> is displayed

4. Visit http://texturizer.net/firefox/
5. focus url bar, type 't', arrow down to texturizer entry
6. t<highlight>tp://texturizer.net/firefox/</highlight> is displayed

7. Visit http://prefbar.mozdev.org/
8. focus url bar, type 'p', arrow down to prefbar entry
9. p<highlight>://prefbar.mozdev.org/</highlight> is displayed

10. Visit http://www.mozilla.org/
11. focus url bar, type 'm', arrow down to mozilla entry
12. m<highlight>ozilla.org/</highlight> is displayed

Clearly we're messing things up here - we're looking for the first instance of
'h', 't' and 'p' in the entire url when displaying rather than the first
following ://, which is probably what we should do.

According to bug 202992 this creates a problem with the autofill pref, but I
haven't been able to verify this yet.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
--> Ben
Assignee: hewitt → bugs
Status: REOPENED → NEW
OS: Windows XP → All
(In reply to comment #21)
> This is apparently still causing problems (see duped bug 202992).
... 
> According to bug 202992 this creates a problem with the autofill pref, but I
> haven't been able to verify this yet.

Umm, duh, wrong bug... the dupe is bug 242747. Time to head to bed methinks.
Sorry for the bugspam.
Flags: blocking1.0+ → blocking1.0-
Still seeing this bug:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040816 Firefox/0.9.1+
See also core bug 266763 and its dependencies.
See

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp&rev=1.29&mark=1146-1151#1141

for the code that's behind this.
Assignee: bugs → nobody
Component: Location Bar and Autocomplete → Autocomplete
Flags: blocking-aviary1.0-
Product: Firefox → Toolkit
QA Contact: davidpjames → nobody
Hardware: PC → All
*** Bug 269421 has been marked as a duplicate of this bug. ***
*** Bug 266763 has been marked as a duplicate of this bug. ***
Bug 122862 is the Seamonkey version.
Depends on: 122862
Summary: "h" or "htt" cut off in address bar autocomplete → "h" or "htt" cut off in address bar autocomplete (adds :// to domain names starting with "p" or "t")
*** Bug 304916 has been marked as a duplicate of this bug. ***
This naive patch against CVS makes location bar autocompletion skip
"://"-suffixed schemes unless they are explicitly searched for. I haven't
considered other potential users of nsAutoCompleteController::CompleteValue().

My testing was with Firefox 1.0.6 (the patch applies at a -100 line offset).

P.S. I've prepared a similar patch for xpfe/.../autocomplete.xml if this is of
value.
Attachment #195013 - Flags: first-review?
Attachment #195013 - Flags: first-review? → first-review?(mconnor)
*** Bug 310340 has been marked as a duplicate of this bug. ***
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050928
Firefox/1.4 ID:2005092805

WFM.
(In reply to comment #33)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050928
> Firefox/1.4 ID:2005092805
> 
> WFM.
This is still broken.  It happens when you type just a P or a T in the location
bar and then press the down arrow you then can see the chopped url in the
location bar. 
e.g. ttp://tinderbox.mozilla.org/Firefox/
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050928
Firefox/1.4 ID:2005092806


*** Bug 311630 has been marked as a duplicate of this bug. ***
Attachment #195013 - Flags: first-review?(mconnor) → first-review+
Assignee: nobody → toms
Checked in on the trunk. Thanks for the patch Tom, and sorry for the delay in getting it checked in!

mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp;
new revision: 1.37; previous revision: 1.36
Status: NEW → RESOLVED
Closed: 20 years ago → 18 years ago
Flags: blocking-aviary2?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha1
Attachment #195013 - Flags: approval1.8.1?
Attachment #195013 - Flags: approval1.8.0.1?
Comment on attachment 195013 [details] [diff] [review]
Skip "://"-suffixed scheme unless explicitly searched for (bug 202992).

a- for 1.8.0.1, you don't (yet) need approvals for ff2 (1.8.1)
Attachment #195013 - Flags: approval1.8.0.1? → approval1.8.0.1-
Attachment #195013 - Flags: approval1.8.1? → approval1.8.1+
Flags: blocking-firefox2? → blocking-firefox2+
Fixed on the 1.8 branch for Firefox 2.0.
mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp;
new revision: 1.32.2.3;
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha1 → mozilla1.8.1
Version: unspecified → 1.8 Branch
Depends on: 323549
Did this cause bug 323549?
(In reply to comment #39)
> Did this cause bug 323549?

Yes, confirmed by local backout.
*** Bug 326505 has been marked as a duplicate of this bug. ***
*** Bug 329576 has been marked as a duplicate of this bug. ***
The fix for bug 323549 reverted this patch since it caused other problems.  This was then fixed in a different way for that bug (don't go through nsAutoCompleteController::CompleteValue() at all).  However, that means that the problem is still visible for folks who have browser.urlbar.autoFill = true.  I have an idea of how to fix this that I intend to try to write a patch for in the next couple of days:

(1) Check if the user's string matches the beginning of the desired autocomplete target; if so autocomplete to the target, selecting the remainder, and exit (functionally identical to the current code)
(2) If not, then see if we're autocompleting to a URI.  If not, just do the current behavior (since this code is shared by more places than just the urlbar autocomplete); however, if so:
(3) See if "http://" + the user's string matches the beginning of the desired autocomplete target; if so autocomplete and return.
(4) If not, then fail to autocomplete (do nothing), and return.

We reach case (4) if we're autocompleting, say, www.google.com when the user typed "g", or addresses at a different protocol than http://.  In this case, the only way to autocomplete the string the user types in correctly would be to prefix it with something (for example, "http://www." or "ftp://"), and this creates its own set of usability problems that are best avoided.  (For example, the user has "http://foo" and "ftp://food" as autocomplete targets, and tries to type "food"; we'll insert "http://" to match the "foo" case, and then the "food" case won't match at all.)  It might be possible to work around this by checking to see if there is more than one possible autocomplete target, but even that is questionable, and at least my proposed design should fix the actual bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Taking, since I intend to fix this.
Assignee: toms → pkasting
Status: REOPENED → NEW
> (2) If not, then see if we're autocompleting to a URI.  If not, just do the
> current behavior (since this code is shared by more places than just the urlbar
> autocomplete); however, if so:

This is really a fundamental problem that can show up with any autocomplete source (history, addressbook, etc) that returns matches where the search string isn't at the start of the matched string.  If we try to fix it inside autocomplete, we'll just be adding numerous hacks for different cases.  One way around it is bug 325842, although that might not be ideal for URLs.  A more robust fix would be to extend the autocomplete API to include the information about where in the target string the search string was found.
I agree that the problem is fundamental.  I don't think the fix in Bug 325842 is appropriate for URIs in most cases (certainly not in the urlbar.autoFill case).  I'm not sure how knowing where the target string was found helps us to do better behavior (since we can, and do, just FindInReadable() on it again).

I'm testing a patch for this that's similar to my proposed design (with some code cleanup).  While I agree that in some sense fixing this inside autocomplete is "adding a hack for a different case", fundamentally, I think it's correct that autocomplete know that "foo.com" is a valid autocompletion for "http://foo.com" but not for "ftp://foo.com".

Additionally, my patch doesn't preclude the fix in Bug 325842 from being implemented, as that's fairly orthogonal.  Presumably it would get implemented in the non-URI case, which, as I said above, seems better for me, since I'm not convinced that makes sense for URIs.
> I'm not sure how knowing where the target string was found helps us to do 
> better behavior (since we can, and do, just FindInReadable() on it again).

If we knew where the target string was located, we wouldn't call FindInReadable.
OK, here's a patch that fixes the autocomplete component to not be as aggressive in completing URIs from the middle of the URI.  This also attempts to clean up the code some and add comments; unfortunately that means the patch is a little bigger and less-clear than it otherwise might be.

There are several effects of this patch if you have browser.urlbar.autoFill set to true:
(1) If you have "http://prefbar.mozilla.org/" in your autocomplete list, typing "p" in the address bar fills to "prefbar.mozilla.org/" instead of "p://prefbar.mozilla.org/".
(2) If you have "http://www.foo.com/" in your autocomplete list, typing "f" in the address bar does not autofill, instead of filling to "foo.com".  HOWEVER, the drop-down autocomplete list will still show you "http://www.foo.com/"; you just won't get it autofilled in the address bar anymore.  To me this is a bugfix (since not all sites have foo.com and www.foo.com mapped to the same thing, and we shouldn't assume "foo.com" is a valid autocompletion when we've only seen "www.foo.com" in the past).
(3) If you have "ftp://www.foo.com/" in your autocomplete list, typing "www.f" in the address bar does not autofill, instead of filling to "www.foo.com/".  Again, "ftp://www.foo.com/" will still be shown in the drop-down list, and I consider this a bugfix (for similar reasons as (2)).
(4) If you have "file://something" in your autocomplete list, typing "file://s" will still autofill as expected (that is, this patch does not cause Bug 323549 to reappear in any way).
(5) For non-URIs (which I define as "things that don't seem to have a scheme"), this patch does not change existing behavior, but I have added a comment that the existing behavior is questionable.

This patch does not preclude Bug 325842 from being fixed, although it will bit-rot the current patch on that bug.

In response to comment 47, some of the logic in this routine could be simplified if we already knew where the string was located.  However, it's not a huge deal in terms of code complexity or speed, so I'm not too worried about that ATM.
Attachment #195013 - Attachment is obsolete: true
Attachment #218910 - Flags: first-review?(bryner)
I think this looks like the right behavior. This code also looks fine to me, but if there's somebody that has more experience with the autocomplete controller, I'd be interested in hearing what they think.
Attachment #218910 - Flags: first-review?(bryner) → first-review+
Attachment #218910 - Flags: second-review+
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago → 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
*** Bug 336876 has been marked as a duplicate of this bug. ***
Comment on attachment 218910 [details] [diff] [review]
First cut at fix for browser.urlbar.autoFill == true users.

I'd like to get this patch into the branch whenever it's appropriate (don't know if I should be doing it now or waiting until after a3).
Attachment #218910 - Flags: approval-branch-1.8.1?(bryner)
Comment on attachment 218910 [details] [diff] [review]
First cut at fix for browser.urlbar.autoFill == true users.

no reason to wait until a3 on this...
Attachment #218910 - Flags: approval-branch-1.8.1?(bryner) → approval-branch-1.8.1+
Fixed on branch.

/cvsroot/mozilla/toolkit/components/autocomplete/src/Makefile.in,v  <--  Makefile.in
new revision: 1.5.8.4; previous revision: 1.5.8.3

/cvsroot/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp,v  <--  nsAutoCompleteController.cpp
new revision: 1.32.2.7; previous revision: 1.32.2.6
Keywords: fixed1.8.1
*** Bug 343215 has been marked as a duplicate of this bug. ***
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.