92132 - Typing in path to URLbar loads via file:, adds to history as http [FIX]

Status: RESOLVED FIXED

(Core Graveyard :: History: Global, defect, P2)

Description

[Jeremy M. Dolan]

If I type in "/home/jmd" in the URL bar and hit entry, file:///home/jmd/ loads,
as it should. However, in the history dropdown, it shows up as http:///home/jmd
(yes, three slashes too), which doesn't work.

Comment 1

[Jeremy M. Dolan]

see also 99855 re: URL guessing (www.   .com)

Comment 2

[Alec Flett]

argh.. take two at reassigning history bugs to new owner

Comment 3

[Chase Tingley]

*** Bug 103500 has been marked as a duplicate of this bug. ***

Comment 4

[Pete Collins (MDG)]

I'll take this one. Marking target 0.9.8

--pete

Comment 5

[Bradley Baetz (:bbaetz)]

*** Bug 115990 has been marked as a duplicate of this bug. ***

Comment 6

[Igor Nekrestyanov]

This bug was introduced by fix to bug 73845.

The problem is that we are adding http scheme to url 
if we can not extract scheme from it.
I think this solution was introduced because there was 
complains on problems in netwerk code if scheme is null. 

However, urlbar history is supposed to store _exactly_ 
what was typed in by user. Therefore i propose to 
remove this default initialization. 
This is safe because we do not pass this url 
to netwerk code - we only want to compare it with 
already stored urls to detect dupe (if any).

Other possibility is to
save original string from urlbar and add it to the history 
instead of adding of probably modified one. 
But this seems to be overkill.

I'll attach the patch shortly - please review.
 

Comment 7

[Igor Nekrestyanov]

Attachment: Proposed patch

Comment 8

[Pete Collins (MDG)]

Attachment: or . . .

Comment 9

[Pete Collins (MDG)]

Either of these patches works fine by me. 

I'll r=attachment 62220 [details] [diff] [review] if we want to keep what the user types. 

--pete

Comment 10

[Igor Nekrestyanov]

pete: it looks like your patch does not really fix the problem.
      it only change it. For example if you type www.mozilla.org in urlbar 
      then it will be stored as file:// ? Also, it does not fix 
      problems reported in dupes for this bug.

Anyway, i realized that my patch also is not best. Alternative approach 
i mentioned (to save original string and add it) seems preferrable because
 1) it detects http://www.mozilla.org and www.mozilla.org are dupes
 2) it keeps usage of nsIIOService consistent with other places :)

I will attach new version of patch.

      

Comment 11

[Igor Nekrestyanov]

Attachment: Alternative patch

Also fixes problems from bug 115990 and bug 99855

Comment 12

[Pete Collins (MDG)]

This looks and works good for me. 

(From update of attachment 62368 [details] [diff] [review])
r=pete

Comment 13

[Pete Collins (MDG)]

Igor, i am trying to get you an sr=.

I can check this in for you if need be.

--pete

Comment 14

[Igor Nekrestyanov]

Last suggested fix also should fix the problem reported in bug 57932.

Comment 15

[Alexei V. Mokeev]

Add status whiteboard keyword

Comment 16

[Pete Collins (MDG)]

Hewitt! sr me bro!

A two line patch here!


Thanks

--pete

Comment 17

[Olav Vitters]

*** Bug 118435 has been marked as a duplicate of this bug. ***

Comment 18

[Brendan Eich [:brendan]]

Pete, I'm delegating sr= to alecf.  Alec, can you field this one?  Thanks.

/be

Comment 19

[Alec Flett]

I want an r=radha here before I sr=

Comment 20

[Pete Collins (MDG)]

radha, can you accommodate us here?
This is an easy fix.

Thanks

--pete

Comment 21

[Alec Flett]

just looked at this code again - this is urlbar history, and not global
history... "history" is vague and could mean global, urlbar or session - there
is a big difference - that is why I asked radha to do the review, I know global
history well but don't know urlbar history at all. 
(I was in fact confused when I changed it before to session history)

Comment 22

[Pete Collins (MDG)]

So alec, can my r= be valid in this case to expediate things along?

--pete

Comment 23

[Alec Flett]

and I repeat:
> that is why I asked radha to do the review, I know global
> history well but don't know urlbar history at all. 

I want radha to do the review.

Comment 24

[Radha on family leave (not reading bugmail)]

The first patch looks OK to me(62220). But I would like you to get a review from
its current owner (blake ross or jaggernaut). I think the first patch  *may*
fundamentally change the way urlbar history does autocomplete. If the
autocomplete code assumes the existence of a protocol (default http://) in all
the rdf entries, then the proposed patch may break certain autocompletion, like
"www.moz" typed into the urlbar may not autocomplete with
"http://www.mozilla.org " already present in the database. 

Comment 25

[Pete Collins (MDG)]

If you actually apply the patch, you will see that it doesn't:

> break certain autocompletion, like
> "www.moz" typed into the urlbar may not autocomplete with
> "http://www.mozilla.org " already present in the database. 

I've been using mozilla w/ this patch for weeks now. urlbar history finally does
what it should.

--pete

Comment 26

[Pete Collins (MDG)]

The working patch to be reviewd here is attachment #62368 [details] [diff] [review].

--pete

Comment 27

[jag (Peter Annema)]

Comment on attachment 62368 [details] [diff] [review]
Alternative patch

r=/sr=jag

Comment 28

[Pete Collins (MDG)]

checked in marking FIXED


--pete