1420395 - Firefox doesn't allow navigating to fully IDN domains in the URL bar without including "http(s)://" because of broken uri fixup code (was: url bar issues with uris with ".გე" suffix)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[brunjadzem]

What did you do?
================
1. Type რეგისტრაცია.გე in address bar
2. Press Enter

What happened?
==============
Firefox made search request

What should have happened?
==========================
Perform DNS lookup and show valid web page for given domain

Is there anything else we should know?
======================================
https://features.icann.org/delegation-%E1%83%92%E1%83%94-ge-domain-representing-georgia-georgian-mkhedruli-script-information-technologies

Comment 1

[Junior [inactive]]

Not a DNS issue, since we can navigate the page by a hyperlink to http://რეგისტრაცია.გე

Ni? Gijs who might have some idea about it.

Comment 2

[:Gijs (he/him)]

This is because we enter this code block:

https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/docshell/base/nsDefaultURIFixup.cpp#951-957

The check for  host.EqualsIgnoreCase(asciiHost.get()) is supposed to catch the IDN case here (ie we shouldn't be entering that block), and it's not, because bug 1380617 changed uri.host to *also* return punycode.

Valentin, can you take a look?

Comment 3

[:Gijs (he/him)]

Workaround for users who use these domains lots: flip the pref browser.fixup.dns_first_for_single_words

Unfortunately this will significantly affect perceived performance of the address bar for single-word entries on Windows, so we can't really do this everywhere.

Comment 4

[:Gijs (he/him)]

Actually, I can probably just write the short-term patch here. Though having looked at this code again, perhaps it needs a refactor... but not here/now.

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1420395 - deal with IDN domains without protocols correctly in nsIURIFixup,

Review commit: https://reviewboard.mozilla.org/r/203516/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203516/

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8932470 [details]
Bug 1420395 - deal with IDN domains without protocols correctly in nsIURIFixup,

https://reviewboard.mozilla.org/r/203516/#review208994

::: docshell/base/nsDefaultURIFixup.cpp:927
(Diff revision 1)
>      return NS_OK;
>    }
>  
> -  nsAutoCString asciiHost;
>    nsAutoCString host;
> +  nsAutoCString displayHost;

I think it would be clearer if we called them asciiHost and displayHost. It would also make the behaviour unaffected by pref changes.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8932470 [details]
Bug 1420395 - deal with IDN domains without protocols correctly in nsIURIFixup,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203516/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/977a731c6f05
deal with IDN domains without protocols correctly in nsIURIFixup, r=valentin

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/977a731c6f05

Comment 10

[:Gijs (he/him)]

Reporter, would it be possible for you to verify this is fixed for you on our nightly (experimental) branch? You can download nightly from https://nightly.mozilla.org/, and/or verify that your nightly build is updated to a build dated 2017-11-29. macOS builds for today have already finished and are available as updates. If you want to test Linux or Windows, they should be available later today (probably in about an hour or so).

Once we verify this is fixed on nightly, we can uplift the fix for beta so it'll go out with 58. I'll see with release management if there's an opportunity to make this ride-along with any 57 dot-releases if we'll still end up doing those.

Comment 11

[brunjadzem]

(In reply to :Gijs from comment #10)
> Reporter, would it be possible for you to verify this is fixed for you on
> our nightly (experimental) branch? You can download nightly from
> https://nightly.mozilla.org/, and/or verify that your nightly build is
> updated to a build dated 2017-11-29. macOS builds for today have already
> finished and are available as updates. If you want to test Linux or Windows,
> they should be available later today (probably in about an hour or so).
> 
> Once we verify this is fixed on nightly, we can uplift the fix for beta so
> it'll go out with 58. I'll see with release management if there's an
> opportunity to make this ride-along with any 57 dot-releases if we'll still
> end up doing those.

Hello, I've tested it on latest Nightly and issue seems to be solved. Thanks.

Comment 12

[:Gijs (he/him)]

Comment on attachment 8932470 [details]
Bug 1420395 - deal with IDN domains without protocols correctly in nsIURIFixup,

Approval Request Comment
[Feature/Bug causing the regression]: bug 1380617
[User impact if declined]: users can't easily use typing in the url bar to navigate to IDN domains
[Is this code covered by automated tests?]: yes in general, IDN urls as part of this patch
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple swapping of 1 thing we're comparing (which used to be unicode and is now ascii) to another thing (which is now definitely unicode) which reinstates old behaviour, plus a test to verify we don't break this again.
[String changes made/needed]: no


Ritu: what's the prognosis on doing another 57 dot-release and if we did, do you want this as a ride-along? I can ask for release approval if so. I'm pretty confident in the patch, but not sure what our plans for 57 are at this point.

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8932470 [details]
Bug 1420395 - deal with IDN domains without protocols correctly in nsIURIFixup,

Fix a broken IDN domains navigation issue. Beta58+.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/27dc691eb4a0

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Gijs, there are no plans yet for another dot release that will include a few more ride-alongs such as this one. I will still add this to my tracking list and we can evaluate the risk-reward when the time comes. It will also be good to bake this in Nightly and Beta beforehand, glad to see that already happen.

Comment 16

[Julien Cristau [:jcristau] (back April 22)]

It seems very unlikely we'll have a 57.0.3

Comment 17

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 59.0a1 (2017-11-24) on Windows 10, 64 Bit!

This bug's fix is verified with latest release and latest developer edition!

Build ID  : 20180206200532
Use Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

Build ID  : 20180301022608
Use Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0

Comment 18

[Md. Majedul islam]

I have reproduced this bug with Nightly 59.0a1(2017-11-24) on ubuntu 16.04, 64 Bit!

This bug's fix is verified with latest release and latest developer edition!

Build ID : 20180208175058
Use Agent : Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0


Build ID : 20180301022608
Use Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0

[testday-20180302]