Closed
Bug 1257877
Opened 8 years ago
Closed 8 years ago
Remove support for UTF-16 from TextEncoder
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: hsivonen, Assigned: emk)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed, site-compat)
Attachments
(2 files, 1 obsolete file)
22.40 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
9.17 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
https://github.com/whatwg/encoding/commit/8360f775c8df145f649047c7d59c5ff733ade112 removed support for UTF-16 in TextEncoder. We should make our implementation reflect the spec change.
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=079860900d30
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8732514 -
Flags: review?(Ms2ger)
Updated•8 years ago
|
Keywords: dev-doc-needed
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8732513 [details] [diff] [review] patch Review of attachment 8732513 [details] [diff] [review]: ----------------------------------------------------------------- Thank you! (I'm a bit uneasy about not throwing if there's an argument to the constructor that's not an ASCII case-insensitive match for the string string "utf-8", but that's really a spec concern, since the patch implements the spec.)
Attachment #8732513 -
Flags: review?(hsivonen) → review+
Comment 5•8 years ago
|
||
Comment on attachment 8732514 [details] [diff] [review] wpt changes Review of attachment 8732514 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/encoding/api-basics.html @@ +14,5 @@ > assert_array_equals(new TextEncoder().encode(undefined), [], 'input default should be empty string') > }, 'Default inputs'); > > > function testEncodeDecodeSample(encoding, string, bytes) { Do we need this function if it has just one caller? ::: testing/web-platform/tests/encoding/textencoder-constructor-non-utf.html @@ +17,5 @@ > } else { > test(function() { > assert_equals(new TextDecoder(encoding.name).encoding, encoding.name); > + assert_equals(new TextEncoder(encoding.name).encoding, 'utf-8'); > + }, 'Non-UTF-8 encodings supported only for decode, not encode: ' + encoding.name); I'd prefer section.encodings.forEach(function(encoding) { if (encoding.name !== 'replacement') { test(function() { assert_equals(new TextDecoder(encoding.name).encoding, encoding.name); }, "Encoding argument supported for decode: ' + encoding.name); } test(function() { assert_equals(new TextEncoder(encoding.name).encoding, 'utf-8'); }, "Encoding argument not considered for encode: ' + encoding.name); });
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to :Ms2ger from comment #5) > ::: testing/web-platform/tests/encoding/api-basics.html > @@ +14,5 @@ > > assert_array_equals(new TextEncoder().encode(undefined), [], 'input default should be empty string') > > }, 'Default inputs'); > > > > > > function testEncodeDecodeSample(encoding, string, bytes) { > > Do we need this function if it has just one caller? Maybe not. Merged with the caller. > ::: testing/web-platform/tests/encoding/textencoder-constructor-non-utf.html > @@ +17,5 @@ > > } else { > > test(function() { > > assert_equals(new TextDecoder(encoding.name).encoding, encoding.name); > > + assert_equals(new TextEncoder(encoding.name).encoding, 'utf-8'); > > + }, 'Non-UTF-8 encodings supported only for decode, not encode: ' + encoding.name); > > I'd prefer > > section.encodings.forEach(function(encoding) { > if (encoding.name !== 'replacement') { > test(function() { > assert_equals(new TextDecoder(encoding.name).encoding, > encoding.name); > }, "Encoding argument supported for decode: ' + encoding.name); > } > > test(function() { > assert_equals(new TextEncoder(encoding.name).encoding, 'utf-8'); > }, "Encoding argument not considered for encode: ' + encoding.name); > }); Done.
Attachment #8732514 -
Attachment is obsolete: true
Attachment #8732514 -
Flags: review?(Ms2ger)
Attachment #8733876 -
Flags: review?(Ms2ger)
Comment 7•8 years ago
|
||
Comment on attachment 8733876 [details] [diff] [review] wpt changes, v2 Review of attachment 8733876 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #8733876 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=201d62214f46
Assignee | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd3799dfd531b8a72becc4c2c78128fd052d8818 Bug 1257877 - Remove UTF-16 support from TextEncoder. r=hsivonen https://hg.mozilla.org/integration/mozilla-inbound/rev/fbb40278805ff5a8aac8b1a1dbd8b0a180f82296 Bug 1257877 - Remove UTF-16 support from TextEncoder tests to reflect a spec change. r=Ms2ger
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cd3799dfd531 https://hg.mozilla.org/mozilla-central/rev/fbb40278805f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 11•8 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/textencoder-no-longer-supports-utf-16/
Keywords: site-compat
Comment 12•8 years ago
|
||
For the MDN writer: this patch also removed the optional parameter to the constructor, as well as changing the supported values for the encode() method.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•