Closed Bug 1380617 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: DOM: Core & HTML, enhancement)

x86
macOS
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: addon-compat, dev-doc-needed, Whiteboard: [necko-active] btpp-active)

Attachments

(9 files, 1 obsolete file)

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
+++ 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.
Depends on: 1380932
MozReview-Commit-ID: 4EbA4QJuNIX
Attachment #8886490 - Flags: review?(gijskruitbosch+bugs)
- 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)
Attachment #8886482 - Flags: review+
MozReview-Commit-ID: H6vFyq6SdiJ
Attachment #8886538 - Flags: review?(mcmanus)
Attachment #8886538 - Flags: review?(mcmanus) → review+
Attachment #8886490 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8886497 - Flags: review?(gijskruitbosch+bugs) → review+
- 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)
Attachment #8887567 - Flags: review+
Updated the patch to use try-catch. Otherwise it would fail for non-URLs
Attachment #8886497 - Attachment is obsolete: true
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+
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
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..
(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 ;-)
(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.
Depends on: 1391421
Depends on: 1400023
Depends on: 1420395
Component: DOM → DOM: Core & HTML
Depends on: 1653201
You need to log in before you can comment on or make changes to this bug.