Last Comment Bug 700566 - pressing enter or copying URL on an http:// site whose hostname begins with "ftp<digit>" results in incorrect URL being loaded/copied
: pressing enter or copying URL on an http:// site whose hostname begins with "...
Status: VERIFIED FIXED
[qa!]
: regression, verified-aurora, verified-beta
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 9 Branch
: x86_64 Linux
: -- normal (vote)
: Firefox 11
Assigned To: Masatoshi Kimura [:emk]
:
Mentors:
Depends on:
Blocks: 665580
  Show dependency treegraph
 
Reported: 2011-11-07 22:34 PST by Matthew McPherrin [:mcpherrin]
Modified: 2011-12-02 05:11 PST (History)
7 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
fixed
fixed


Attachments
Treat also ftp\d+ as FTP host. (2.45 KB, patch)
2011-11-09 05:24 PST, Masatoshi Kimura [:emk]
gavin.sharp: review+
Details | Diff | Splinter Review
Treat also ftp\d+ as FTP host, for check in (3.04 KB, patch)
2011-11-09 14:46 PST, Masatoshi Kimura [:emk]
christian: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Matthew McPherrin [:mcpherrin] 2011-11-07 22:34:18 PST
If http site redirects to an FTP download, the url bar doesn't show ftp:// protocol.

If you copy the url from the bar, it does include the ftp:// though.

STR:
http://ftp1.digi.com/pub/shareware/unix/agetty/agetty21.tar
Click cancel on download prompt.
Note url bar doesn't show ftp://
Copy url. Note it is ftp://ftp1.digi.com/pub/shareware/unix/agetty/agetty21.tar
Comment 1 Masatoshi Kimura [:emk] 2011-11-08 21:45:14 PST
http://ftp1.digi.com/pub/shareware/unix/agetty/agetty21.tar
responded the download content directly without redirect.
How did you conclude the response was redirected?
Comment 2 Matthew McPherrin [:mcpherrin] 2011-11-08 21:52:04 PST
I concluded that because when I copied the url, it was ftp://ftp1.digi.com/pub/shareware/unix/agetty/agetty21.tar instead of an http:// link that I had typed into the address bar.
Comment 3 Masatoshi Kimura [:emk] 2011-11-08 23:48:25 PST
Confirmed. It just copies a wrong URL, not a redirect bug.
To reproduce, I had to type the http:// URL in the location bar (clicking the link didn't leave the URL on the location bar).
Comment 4 Masatoshi Kimura [:emk] 2011-11-09 00:01:54 PST
The problem is that nsDefaultURIFixup and utilityOverlay.js don't agree about what URI is likely to be FTP.
https://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDefaultURIFixup.cpp#530
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/utilityOverlay.js#637
Comment 5 Masatoshi Kimura [:emk] 2011-11-09 05:24:26 PST
Created attachment 573169 [details] [diff] [review]
Treat also ftp\d+ as FTP host.
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-09 14:32:29 PST
Comment on attachment 573169 [details] [diff] [review]
Treat also ftp\d+ as FTP host.

You should adjust the comment above the line that you're changing in trimURL ("ftp\d*\."), and probably just copy the comment from urlbarBindings.xml's trimValue about the function not changing the URL such that calling createFixupURI will produce a different result.
Comment 7 Masatoshi Kimura [:emk] 2011-11-09 14:46:05 PST
Created attachment 573335 [details] [diff] [review]
Treat also ftp\d+ as FTP host, for check in

Thanks. Adjusted and add comments.
Try submission result (before changing comments):
https://tbpl.mozilla.org/?tree=Try&rev=0ba67c58de78
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-10 14:13:28 PST
Thanks for the patch, Kimura-san!
Comment 10 Marco Bonardo [::mak] 2011-11-11 02:14:41 PST
https://hg.mozilla.org/mozilla-central/rev/6194fd540e16
Comment 11 Masatoshi Kimura [:emk] 2011-11-12 01:49:19 PST
Comment on attachment 573335 [details] [diff] [review]
Treat also ftp\d+ as FTP host, for check in

Requesting approval for branches because this is a regression since Firefox 7.
Comment 12 christian 2011-11-15 13:37:20 PST
Comment on attachment 573335 [details] [diff] [review]
Treat also ftp\d+ as FTP host, for check in

[triage comment]
Even though the reward looks low, the risk looks low as well. Approved for beta and aurora. Please land as soon as possible.
Comment 14 Virgil Dicu [:virgil] [QA] 2011-12-02 05:11:29 PST
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0 Firefox Beta4
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111201 Firefox/10.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111202 Firefox/11.0a1

Verified on Ubuntu 11.10 using F9b4, Firefox Aurora and Nightly using the STR from comment 0. The Url is no longer trimmed for this url and thus the URl is copied appropriately.

Note You need to log in before you can comment on or make changes to this bug.