1322874 - Remove nsIURI::originCharset

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Henri Sivonen (:hsivonen)]

It seems that nsIURI::originCharset has two use cases:
 1) Dealing with the spec-incompliant feature of escapes in the hash (reference) part of the URL.
 2) For UI display of non-UTF-8 URLs.

If per bug 817374 comment 18 we no longer care to have the complexity of having pretty display of query strings on legacy-encoded pages and if our hash part handling is reaching spec compliance, it seems we could remove nsIURI::originCharset.

Comment 1

[Masatoshi Kimura [:emk]]

If I read the source correctly, we already stop decoding the hash part.

GettersDecodeURLHash() will return true only if the "dom.url.getters_decode_hash" is true.[1][2]
"dom.url.getters_decode_hash" is false by default[3] and we have no UI to flip this pref (except about:config).

[1] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/dom/base/nsContentUtils.h#2079-2082
[2] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/dom/base/nsContentUtils.cpp#586-587
[3] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/modules/libpref/init/all.js#220

Comment 2

[Masatoshi Kimura [:emk]]

And I would like to keep the first parameter until we drop support for XUL/XPCOM addons.

Comment 3

[Masatoshi Kimura [:emk]]

Oh, I didn't read the bug title correctly.

Comment #2 was about the first parameter of unEscapeURIForUI.

Comment 4

[u408661]

Valentin, I seem to remember you having some hands in this. Can you take a look and re-triage or work on as appropriate? Thanks.

Comment 5

[Masatoshi Kimura [:emk]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=12d81953a612c84fc51fed05e6219c516714b039
https://treeherder.mozilla.org/#/jobs?repo=try&revision=76569857fd6d13cd5f6ddc8da83d8f23ce0e0703

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Bug 1322874 - Get rid of nsIURI.originCharset. .gosu

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

Comment 7

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

Comment on attachment 8893231 [details]
Bug 1322874 - Get rid of nsIURI.originCharset. .gosu

https://reviewboard.mozilla.org/r/164270/#review169736

Just a few minor nits. 
r=me Thanks!

::: commit-message-4c731:3
(Diff revision 1)
> +Bug 1322874 - Get rid of nsIURI.originCharset. r?valentin.gosu
> +
> +MozReview-Commit-ID: 3tHd0VCWSqF

Please add a detailed description message:
* what originCharset was used for
* why we don't need it anymore

::: commit-message-4c731:3
(Diff revision 1)
> +Bug 1322874 - Get rid of nsIURI.originCharset. r?valentin.gosu
> +
> +MozReview-Commit-ID: 3tHd0VCWSqF

Please add a detailed description message:
* what originCharset was used for
* why we don't need it anymore

::: netwerk/base/nsStandardURL.cpp:3597
(Diff revision 1)
>      if (NS_FAILED(rv)) return rv;
>  
>      rv = ReadSegment(stream, mRef);
>      if (NS_FAILED(rv)) return rv;
>  
> -    rv = NS_ReadOptionalCString(stream, mOriginCharset);
> +    nsAutoCString old_originCharset;

nit: let's not mix camelCase with snake_case :)

Comment 8

[Masatoshi Kimura [:emk]]

Comment on attachment 8893231 [details]
Bug 1322874 - Get rid of nsIURI.originCharset. .gosu

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ff32bdb356d
Get rid of nsIURI.originCharset. r=valentin.gosu

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ff32bdb356d