Make nsStandardURL.host/spec/etc return punycode by default. Fix tests and UI

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: valentin, Assigned: valentin)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {addon-compat, dev-doc-needed})

unspecified
mozilla57
x86
Mac OS X
addon-compat, dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [necko-active] btpp-active)

Attachments

(9 attachments, 1 obsolete attachment)

12.89 KB, patch
valentin
: review+
Details | Diff | Splinter Review
1.39 KB, patch
Gijs
: review+
Details | Diff | Splinter Review
1.25 KB, patch
mcmanus
: review+
Details | Diff | Splinter Review
9.14 KB, patch
Details | Diff | Splinter Review
12.99 KB, patch
Details | Diff | Splinter Review
4.65 KB, patch
valentin
: review+
Details | Diff | Splinter Review
1.69 KB, patch
valentin
: review+
Details | Diff | Splinter Review
6.28 KB, patch
mcmanus
: review+
Gijs
: review+
Details | Diff | Splinter Review
1.83 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
+++ This bug was initially created as a clone of Bug #945240 +++

This bug involves landing the pref change, test changes and UI fixes.
UI features are don't usually have tests for IDNA hostnames, so we should add some.
(Assignee)

Comment 1

2 years ago
Created attachment 8886482 [details] [diff] [review]
Change firefox code to use uri.displaySpec when unicode URLs are wanted

r+d by Gijs in bug 945240 comment 92
(Assignee)

Updated

2 years ago
Depends on: 1380932
(Assignee)

Comment 2

2 years ago
Created attachment 8886490 [details] [diff] [review]
Make sure "Did you mean to go to X" displays unicode domain

MozReview-Commit-ID: 4EbA4QJuNIX
Attachment #8886490 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 3

2 years ago
Created attachment 8886497 [details] [diff] [review]
Make the PageInfo panel return unicode URLs

- fixing the hostName in getWindowInfo fixes the issue across the PageInfo panel
- fixing docInfo.referrer also fixes the Referring URL on the General tab
MozReview-Commit-ID: 9x9uWp2R3Yj
Attachment #8886497 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Updated

2 years ago
Attachment #8886482 - Flags: review+
(Assignee)

Comment 4

2 years ago
Created attachment 8886538 [details] [diff] [review]
Set network.standard-url.punycode-host to true

MozReview-Commit-ID: H6vFyq6SdiJ
Attachment #8886538 - Flags: review?(mcmanus)
Attachment #8886538 - Flags: review?(mcmanus) → review+

Updated

2 years ago
Attachment #8886490 - Flags: review?(gijskruitbosch+bugs) → review+

Updated

2 years ago
Attachment #8886497 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 7

2 years ago
Created attachment 8887556 [details] [diff] [review]
Fix web platform tests meta to expect punycode encoding

r+d by annevk in bug 945240 comment 62
(Assignee)

Comment 8

2 years ago
Created attachment 8887558 [details] [diff] [review]
Fix tests that expect unicode encoding of hostname

r+d by smaug in bug 945240 comment 62
(Assignee)

Comment 9

2 years ago
Created attachment 8887566 [details] [diff] [review]
Fix tests that use nsIURI.host expecting unicode domain name

r+d by mayhemer and smaug in bug 945240 comment 62
(Assignee)

Comment 10

2 years ago
Created attachment 8887567 [details] [diff] [review]
Add test checking that URL.origin returns punycode

r+d by smaug in bug 945240 comment 88
(Assignee)

Comment 11

2 years ago
Created attachment 8887569 [details] [diff] [review]
Add nsIURI.displayPrePath

- Use displayPrePath in the pageInfo permissions that shows "Permissions for:"
- The extra displayPrePath method is necessary because it's difficult to compute it manually, as opposed to not having a displaySpecWithoutRef - as it's easy to get that by truncating displaySpec at the first '#' symbol.

MozReview-Commit-ID: 9RM5kQ2OqfC
Attachment #8887569 - Flags: review?(mcmanus)
Attachment #8887569 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Updated

2 years ago
Attachment #8887567 - Flags: review+
(Assignee)

Comment 12

2 years ago
Created attachment 8887573 [details] [diff] [review]
Make the PageInfo panel return unicode URLs

Updated the patch to use try-catch. Otherwise it would fail for non-URLs
(Assignee)

Updated

2 years ago
Attachment #8886497 - Attachment is obsolete: true
(Assignee)

Comment 13

2 years ago
Comment on attachment 8887566 [details] [diff] [review]
Fix tests that use nsIURI.host expecting unicode domain name

Actually reviewed in bug 945240 comment 41 and 49
Attachment #8887566 - Flags: review+
Attachment #8887569 - Flags: review?(mcmanus) → review+

Updated

2 years ago
Attachment #8887569 - Flags: review?(gijskruitbosch+bugs) → review+
Bug 1385883 means that we should use .displaySpec to access the history database for compatibility with old data. For example:
https://dxr.mozilla.org/mozilla-central/rev/36f95aeb4c77f7cf3b3366583008cd6e4b6b1dba/browser/components/extensions/ext-history.js#97,102,113,122
(Assignee)

Comment 18

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b53d1d68db0e773d7a032225946bcce66a1d40b8
Bug 1380617 - Change firefox code to use uri.displaySpec when unicode URLs are wanted r=Gijs

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6cca2e3574180562901d6c5b529ff3db1c0da8d
Bug 1380617 - Make sure "Did you mean to go to X" displays unicode domain r=Gijs

https://hg.mozilla.org/integration/mozilla-inbound/rev/87bfb9b2974c83c7c7f71cb0587236292c12a47d
Bug 1380617 - Make the PageInfo panel return unicode URLs r=Gijs

https://hg.mozilla.org/integration/mozilla-inbound/rev/82c04a5c89b6bbe4a711a7e6531bc50ec5e48483
Bug 1380617 - Add nsIURI.displayPrePath r=mcmanus,Gijs

https://hg.mozilla.org/integration/mozilla-inbound/rev/19915252f2d4d7701bca41d10fb16dcf87334a7d
Bug 1380617 - Set network.standard-url.punycode-host to true r=mcmanus

https://hg.mozilla.org/integration/mozilla-inbound/rev/9004c524d6e9f8c5f23af5fdd4e57d32dc7be680
Bug 1380617 - Add test checking that URL.origin returns punycode r=smaug

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1a66e455c362eee99bd65480f2fd87094f29f5f
Bug 1380617 - Fix tests that use nsIURI.host expecting unicode domain name r=smaug,honzab

https://hg.mozilla.org/integration/mozilla-inbound/rev/eaef99b59a944630e17fa26387fc4df042306a19
Bug 1380617 - Fix tests that expect unicode encoding of hostname r=smaug

https://hg.mozilla.org/integration/mozilla-inbound/rev/47334606bfa619d5cdc9ff9feeca6cb5f18a73df
Bug 1380617 - Fix web platform tests meta to expect punycode encoding r=annevk
(In reply to Valentin Gosu [:valentin] from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/
> f1a66e455c362eee99bd65480f2fd87094f29f5f
> Bug 1380617 - Fix tests that use nsIURI.host expecting unicode domain name
> r=smaug,honzab

never reviewed this..
(Assignee)

Comment 20

2 years ago
(In reply to Honza Bambas (:mayhemer) from comment #19)
> (In reply to Valentin Gosu [:valentin] from comment #18)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/
> > f1a66e455c362eee99bd65480f2fd87094f29f5f
> > Bug 1380617 - Fix tests that use nsIURI.host expecting unicode domain name
> > r=smaug,honzab
> 
> never reviewed this..

This patch was reviewed in bug 945240 comment 49 - I just moved it here.
We already documented the underlying changes here:

https://developer.mozilla.org/en-US/Firefox/Releases/56#Other

The way I see it, this looks to be part of the same fix; basically just fixing Firefox so that it reports the new URLs correctly when they appear in the UI?

In which case I don't think anything else needs to be communicated to developers on MDN? Let me know if you feel I've got this wrong ;-)
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 25

a year ago
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #24)
> We already documented the underlying changes here:
> 
> https://developer.mozilla.org/en-US/Firefox/Releases/56#Other
> 
> The way I see it, this looks to be part of the same fix; basically just
> fixing Firefox so that it reports the new URLs correctly when they appear in
> the UI?
> 
> In which case I don't think anything else needs to be communicated to
> developers on MDN? Let me know if you feel I've got this wrong ;-)

Actually, these changes have external consequences as well.
Changing the default value of the pref made it so location.href/URL.href/event.origin/etc will now return punycode instead of unicode.
In terms of web-compat these changes make us match the behaviour as all other UAs.
Keywords: dev-doc-complete → dev-doc-needed
(Assignee)

Updated

a year ago
Depends on: 1391421
Depends on: 1400023

Updated

a year ago
Depends on: 1420395
You need to log in before you can comment on or make changes to this bug.