775072 - Firefox rewrites Wiktionary URL's server to wilt.wiktionary

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[André Malafaya Baptista]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

I typed (or edited) the following URL in the address bar:

https://lt.wiktionary.org/wiki/Kategorija:Lakotų_kalba

You have to type or edit. Following a link or simply pasting will not trigger the bug.


Actual results:

The URL was changed to:

https://lt.wilt.wiktionary.org/wiki/Kategorija:Lakotų_kalba

.wilt was added between lt and wiktionary in the URL's server name. The resulst is a "Server not found" error. This only happens in Firefox (my version 14.0.1 on Windows 7 x64)



Expected results:

The URL shouldn't have changed.

Comment 1

[Cork]

Appears to be caused by the autocomplete.

Last good nightly: 2012-06-01
First bad nightly: 2012-06-02

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73783bf75c4c&tochange=5199196b65ec

I'll try to do some regression searches when i get home.

Comment 2

[Cork]

The first bad revision is:
changeset:   95378:960b80d99b4a
user:        Marco Bonardo <[e-mail]>
date:        Fri Jun 01 14:38:39 2012 +0200
summary:     Bug 720081 - Part 2: inline autocomplete should respect protocol and www prefix

Comment 4

[:Gavin Sharp [email: [e-mail]]]

I just saw this with Firefox 14.0.1. I was trying to load http://hg.mozilla.org/mozilla-central/pushloghtml, but every time I entered that text I ended up at "http://hg.mozilhg.mozilla.org/mozilla-central/pushloghtml".

Comment 5

[:Gavin Sharp [email: [e-mail]]]

I managed to visit the page by manually adding a "/" to the end of the autocompleted URL, and after that I can no longer reproduce.

Comment 6

[Marco Bonardo [:mak]]

Pretty sure it's the same bug Margaret reported in #fx-team some time ago.\
Possible culprits:
- trimurl interaction
- SQL function cutting urls
- bad data in db
- nsPlacesAutocomplete.js

In many places we remove a prefix (protocol and www.) and then glue it back, looks like in this case we cut an already trimmed text and put back its first part.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Bug 781427 comment 0 has some suggested STR, but it seems timing-dependent.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I can't (yet) explain comment 4. But for the STR in comment 0 and bug 781427, the issue seems to be due to the call to fixupSearchText() in urlInlineComplete.handleResult (in nsPlacesAutoComplete.js).

For the url:
  https://www.google.com.hk/#hl=zh-CN&newwindow=1&safe=strict&source=hp&q=%E5%95%8A
fixupSearchText() returns:
  google.com.hk/#hl=zh-CN&newwindow=1&safe=strict&source=hp&q=啊

Notice both the missing prefix (expected), and the q= param being unescaped (also expected). However, it then tries to figure out the stripped prefix by using the difference in the two string lengths (which differ by more than just the prefix).
So we end up with a prefix of:
  https://www.google.c

Then eventually we re-combine the prefix and fixed-up URL to get:
  https://www.google.cgoogle.com.hk/#hl=zh-CN&newwindow=1&safe=strict&source=hp&q=%E5%95%8A

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Adding tracking for 16 and wontfixing for 15 since we're a day away from our final beta build for FF15.

Comment 12

[Marco Bonardo [:mak]]

(In reply to Blair McBride (:Unfocused) from comment #9)
> it then tries to figure out the stripped prefix by
> using the difference in the two string lengths (which differ by more than
> just the prefix).

Very good find! I can review the patch once you have it, should not be too hard to add a test to toolkit/components/places/tests/inline/

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Comment 15

[Marco Bonardo [:mak]]

Comment on attachment 654116 [details] [diff] [review]
Patch v1

Review of attachment 654116 [details] [diff] [review]:
-----------------------------------------------------------------

yeah, looks good, I was thinking of alternatives but in the end this is the less invasive.
I assume the test fails without the patch, though didn't try.

thank you!

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Marco Bonardo [:mak] from comment #15)
> I assume the test fails without the patch, though didn't try.

Yep.


https://hg.mozilla.org/integration/fx-team/rev/e1cdfebb423d

Comment 19

[:Gavin Sharp [email: [e-mail]]]

Let's get this on Aurora now, so that we can ship it in 16.

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/e1cdfebb423d

Comment 23

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/633d2043079f

Comment 28

[David Tonhofer]

I may have a completely stupid question here, but I noticed that an extraneous character appeared at the end end of the URL, in my case (bug#791641) an 0x200c = "ZERO WIDTH NON-JOINER"

Isn't this also the case in comment#9? 

Is the size of the returned character array correct or is there, just maybe, a sizing error?

Comment 29

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to dtonhofer from comment #28)
> I may have a completely stupid question here, but I noticed that an
> extraneous character appeared at the end end of the URL, in my case
> (bug#791641) an 0x200c = "ZERO WIDTH NON-JOINER"

This bug would only be triggered if the URL you copied had that character at the end of it already. I believe that explains all of what was observed in bug 791641 (I'd expect it to be stripped when pasting into the command line). Whether or not Firefox should keep that character when another browser strips it is another question, and another bug.