Closed Bug 1486949 Opened 6 years ago Closed 2 years ago

Implement TextEncoderStream and TextDecoderStream

Categories

(Core :: Internationalization, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: ricea, Assigned: saschanaz)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(6 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36 Steps to reproduce: Attempted to use the TextEncoderStream and TextDecoderStream APIs. Actual results: They did not exist. Expected results: They should exist. Standard change: https://github.com/whatwg/encoding/pull/149 Tests: https://github.com/web-platform-tests/wpt/pull/12430
Priority: -- → P3
Blocks: encoding
Status: UNCONFIRMED → NEW
Ever confirmed: true
Type: defect → enhancement
Depends on: 1493537
Depends on: 1730586
No longer depends on: 1493537
Depends on: 1749547
No longer depends on: 1474543

Taking a look.

Assignee: nobody → krosylight

It was a part of the initial skeleton code where the getters threw NOT_IMPLEMENTED. The methods are wrong anyway since .forget() will unset the fields.

This way SetBackpressureChangePromise can be removed which is only exposed for that function.

Depends on D153972

Per the Streams spec, other specs that want to implement custom TransformStream should use GenericTransformStream mixin and store a new TransformStream in a slot. This implements the latter part.

Depends on D153974

Attachment #9288531 - Attachment description: WIP: Bug 1486949 → Bug 1486949 - Part 5: Implement Text{Decoder,Encoder}Stream r=smaug

Other methods probably should do the same, but for now this fulfills the test requirement. The rest is (or should be) tracked by https://bugzilla.mozilla.org/show_bug.cgi?id=1756661.

Depends on D153782

Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9c38a30397bd Part 1: Remove redundant GetReadable/Writable r=smaug https://hg.mozilla.org/integration/autoland/rev/00f43009d419 Part 2: Convert TransformStreamSetBackpressure to a method r=smaug https://hg.mozilla.org/integration/autoland/rev/74ca8f06eb06 Part 3: Refactor TransformerAlgorithms to allow subclassing r=smaug https://hg.mozilla.org/integration/autoland/rev/89137ad4e5c6 Part 4: Implement TransformStream construction for GenericTransformStream r=smaug https://hg.mozilla.org/integration/autoland/rev/91adc5cca2e6 Part 5: Implement Text{Decoder,Encoder}Stream r=smaug https://hg.mozilla.org/integration/autoland/rev/73cb8cf7a3f0 Part 6: Fix ReadableStreamDefaultReader::Read to use the constructor realm r=smaug
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7cb29e4feb27 Part 1: Remove redundant GetReadable/Writable r=smaug https://hg.mozilla.org/integration/autoland/rev/8dd9f5b9e042 Part 2: Convert TransformStreamSetBackpressure to a method r=smaug https://hg.mozilla.org/integration/autoland/rev/862135f6c27f Part 3: Refactor TransformerAlgorithms to allow subclassing r=smaug https://hg.mozilla.org/integration/autoland/rev/f86c3b21e21c Part 4: Implement TransformStream construction for GenericTransformStream r=smaug https://hg.mozilla.org/integration/autoland/rev/57edeeebac29 Part 5: Implement Text{Decoder,Encoder}Stream r=smaug https://hg.mozilla.org/integration/autoland/rev/e3dc48b94eef Part 6: Fix ReadableStreamDefaultReader::Read to use the constructor realm r=smaug

Backed out for causing hazard bustage on TextDecoderStream.cpp

[task 2022-08-12T03:23:47.069Z] TinderboxPrint: rooting hazards<br/>1
[task 2022-08-12T03:23:47.069Z] TinderboxPrint: (unsafe references to unrooted GC pointers)<br/>248
[task 2022-08-12T03:23:47.069Z] TinderboxPrint: (unnecessary roots)<br/>1154
[task 2022-08-12T03:23:47.069Z] TinderboxPrint: missing expected hazards<br/>0
[task 2022-08-12T03:23:47.069Z] TinderboxPrint: heap write hazards<br/>0
[task 2022-08-12T03:23:47.071Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'bufferSource' of type 'mozilla::dom::OwningArrayBufferViewOrArrayBuffer' live across GC call at dom/encoding/TextDecoderStream.cpp:62
[task 2022-08-12T03:23:47.071Z] TEST-UNEXPECTED-FAIL | hazards | 1 rooting hazards detected
[task 2022-08-12T03:23:47.071Z] TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_rooting_hazards_failure'>static rooting hazard analysis failures</a>, visit "Inspect Task" link for hazard details
Flags: needinfo?(krosylight)
Flags: needinfo?(krosylight)

According to glandium, Firefox still runs on at least one big-endian architecture, so the bit that uses a UTF16_LE decoder to read host-endian char16_t data should be ifdefed to use UTF16_BE on big-endian targets.

Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f0358540ed32 Part 1: Remove redundant GetReadable/Writable r=smaug https://hg.mozilla.org/integration/autoland/rev/89263329adf7 Part 2: Convert TransformStreamSetBackpressure to a method r=smaug https://hg.mozilla.org/integration/autoland/rev/5f01a7b13b7b Part 3: Refactor TransformerAlgorithms to allow subclassing r=smaug https://hg.mozilla.org/integration/autoland/rev/0b84f5bf45f3 Part 4: Implement TransformStream construction for GenericTransformStream r=smaug https://hg.mozilla.org/integration/autoland/rev/07a90fe2d68e Part 5: Implement Text{Decoder,Encoder}Stream r=smaug https://hg.mozilla.org/integration/autoland/rev/5bf69788ffb1 Part 6: Fix ReadableStreamDefaultReader::Read to use the constructor realm r=smaug

According to glandium, Firefox still runs on at least one big-endian architecture, so the bit that uses a UTF16_LE decoder to read host-endian char16_t data should be ifdefed to use UTF16_BE on big-endian targets.

Thanks! One more reason I'd be happier if this could be done in encoding-rs side (getting a method receiving char16_t* and supporting pending high surrogate), but for now let's live with this if that's too hard.

Flags: needinfo?(krosylight)

FYI docs work for this was done in https://github.com/mdn/content/issues/20572 - as this was already documented mostly just browser compatibility update.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: