Closed
Bug 1322874
Opened 8 years ago
Closed 7 years ago
Remove nsIURI::originCharset
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: hsivonen, Assigned: emk)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-next])
Attachments
(1 file)
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.
Assignee | ||
Comment 1•8 years ago
|
||
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
Assignee | ||
Comment 2•8 years ago
|
||
And I would like to keep the first parameter until we drop support for XUL/XPCOM addons.
Assignee | ||
Comment 3•8 years ago
|
||
Oh, I didn't read the bug title correctly.
Comment #2 was about the first parameter of unEscapeURIForUI.
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.
Flags: needinfo?(valentin.gosu)
Whiteboard: [necko-next]
Assignee | ||
Updated•7 years ago
|
Blocks: post-57-api-changes
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=12d81953a612c84fc51fed05e6219c516714b039
https://treeherder.mozilla.org/#/jobs?repo=try&revision=76569857fd6d13cd5f6ddc8da83d8f23ce0e0703
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
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 :)
Attachment #8893231 -
Flags: review?(valentin.gosu) → review+
Comment hidden (mozreview-request) |
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/8ff32bdb356d
Get rid of nsIURI.originCharset. r=valentin.gosu
Updated•7 years ago
|
Flags: needinfo?(valentin.gosu)
Comment 10•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•