Support type=module script element #23545

CYBAI · 2019-06-10T14:58:02Z

This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️

Support external module script
Support internal module script
Compile cyclic modules

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix Support type=module script element #23370 (GitHub issue number if applicable)
There are tests for these changes

This change is

CYBAI · 2019-06-10T15:00:23Z

I think there're some tests still failed but I'd like to see if there's any PASS tests with current implementation 🤔

[WIP] Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [ ] Support internal module script - [ ] Compile cyclic modules --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #23370 (GitHub issue number if applicable) - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

bors-servo · 2019-06-10T15:00:30Z

⌛ Trying commit e027b98 with merge b9b7838...

bors-servo · 2019-06-10T15:06:59Z

💔 Test failed - linux-rel-wpt

CYBAI · 2019-06-10T15:28:45Z

@bors-servo try=wpt

sad, I forgot to mark some variable as unused 😂

bors-servo · 2019-06-10T15:28:51Z

⌛ Trying commit 5d7de70 with merge 925aa09...

[WIP] Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [ ] Support internal module script - [ ] Compile cyclic modules --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #23370 (GitHub issue number if applicable) - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

bors-servo · 2019-06-10T15:35:26Z

💔 Test failed - linux-rel-wpt

KiChjang · 2019-06-10T23:53:08Z

components/script/dom/htmlscriptelement.rs

    text: DOMString,
    url: ServoUrl,
    external: bool,
+    type_: ScriptType,


Suggested change

type_: ScriptType,

r#type: ScriptType,

Alternatively,

Suggested change

type_: ScriptType,

script_type: ScriptType,

whichever makes the most sense.

You could also name it ScriptKind instead of ScriptType.

CYBAI · 2019-06-11T02:51:34Z

@bors-servo try=wpt

bors-servo · 2019-06-11T02:51:40Z

⌛ Trying commit 7371b4f with merge 061df76...

[WIP] Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [ ] Support internal module script - [ ] Compile cyclic modules --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #23370 (GitHub issue number if applicable) - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

bors-servo · 2019-06-11T03:15:35Z

💔 Test failed - linux-rel-css

jdm · 2019-06-11T03:37:41Z

You will want to modify https://github.com/servo/servo/blob/master/tests/wpt/include.ini#L94.

CYBAI · 2019-06-11T03:42:41Z

You will want to modify https://github.com/servo/servo/blob/master/tests/wpt/include.ini#L94.

Thanks! Didn't notice that 😂

CYBAI · 2019-06-11T03:43:02Z

@bors-servo try=wpt

Let me know current status!

bors-servo · 2019-06-11T03:43:08Z

⌛ Trying commit 4b234fb with merge c67bdbb...

[WIP] Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [ ] Support internal module script - [ ] Compile cyclic modules --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #23370 (GitHub issue number if applicable) - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

bors-servo · 2019-06-11T04:01:23Z

💔 Test failed - linux-rel-css

Manishearth · 2020-01-06T14:21:29Z

@bors-servo retry try

bors-servo · 2020-01-06T14:21:34Z

⌛ Trying commit 508bfbd with merge 79bddab...

Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [x] Support internal module script - [x] Compile cyclic modules --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23370 (GitHub issue number if applicable) - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

Manishearth · 2020-01-06T14:44:23Z

To keep them in one place, the latest failure is:

0 matches
1 unexpected results that are NOT known-intermittents:
  ▶ Unexpected subtest result in /2dcontext/fill-and-stroke-styles/2d.pattern.paint.repeat.coord2.html:
  │ FAIL [expected PASS] Canvas test: 2d.pattern.paint.repeat.coord2
  │   → assert_equals: Green channel of the pixel at (98, 1) expected 255 but got 0
  │ 
  │ _assertPixel@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:39:5
  │ @http://web-platform.test:8000/2dcontext/fill-and-stroke-styles/2d.pattern.paint.repeat.coord2.html:28:1
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1984:25
  │ _addTest/</<@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:66:15
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1984:25
  └ _addTest/<@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:63:11

1 unexpected results that are NOT known-intermittents:
  ▶ FAIL [expected PASS] /css/CSS2/borders/border-bottom-color-125.xht
  │   → /css/CSS2/borders/border-bottom-color-125.xht 54a9df64f1476dd12020019d7cf22ac34d727bc0
  │   → /css/CSS2/borders/border-bottom-color-021-ref.xht c1172dc374baf74f22c8e9293ef0eba38535a1a6
  └   → Screenshot is solid color 0xFFFFFF for /css/CSS2/borders/border-bottom-color-125.xht

  │ 
  │ 
  └ 
  ▶ TIMEOUT [expected OK] /referrer-policy/gen/top.meta/origin-when-cross-origin/iframe-tag/cross-https.no-redirect.http.html
  │ 
  │ 
  │ 
  └ 
  ▶ TIMEOUT [expected OK] /_webgl/conformance/attribs/gl-vertex-attrib-unconsumed-out-of-bounds.html
  │ 
  │ 
  │ 
  └ 
  ▶ TIMEOUT [expected OK] /_webgl/conformance/canvas/buffer-offscreen-test.html
  │ 
  │ 
  │ 
  └ 
  ▶ TIMEOUT [expected OK] /referrer-policy/gen/req.attr/no-referrer-when-downgrade/a-tag/same-https.no-redirect.http.html
  │ 
  │ 
  │ 
  └ 
  ▶ TIMEOUT [expected OK] /css/css-transitions/properties-value-002.html
  │ 
  │ 
  │ 
  └

Two color failures:

1 unexpected results that are NOT known-intermittents:
  ▶ FAIL [expected PASS] /css/CSS2/borders/border-right-color-044.xht
  │   → /css/CSS2/borders/border-right-color-044.xht 54a9df64f1476dd12020019d7cf22ac34d727bc0
  │   → /css/CSS2/borders/border-right-color-044-ref.xht c695436d4585f381461a01ecc5a8eb6d49f92294
  └   → Screenshot is solid color 0xFFFFFF for /css/CSS2/borders/border-right-color-044.xht

1 unexpected results that are NOT known-intermittents:
  ▶ FAIL [expected PASS] /css/compositing/mix-blend-mode/mix-blend-mode-mask.html
  │   → /css/compositing/mix-blend-mode/mix-blend-mode-mask.html 501a5dfb1fe6e3216f11f8a408f37ffac3e81d32
  │   → /css/compositing/mix-blend-mode/reference/mix-blend-mode-mask-ref.html 54a9df64f1476dd12020019d7cf22ac34d727bc0
  └   → Screenshot is solid color 0xFFFFFF for /css/compositing/mix-blend-mode/reference/mix-blend-mode-mask-ref.html

CYBAI · 2020-01-06T15:00:32Z

IIRC, a bunch of timeouts might be #24762 👀?

Manishearth · 2020-01-06T15:02:12Z

Possibly. The color failures concern me more, we should see if other PRs are having issues as well. Worth double checking this PR to see if any other changes snuck in.

…

On Mon, Jan 6, 2020, 8:30 PM cybai ***@***.***> wrote: IIRC, a bunch of timeouts might be #24762 <#24762> 👀? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#23545?email_source=notifications&email_token=AAMK6SDMBO6ZE2EFAS645P3Q4NBRDA5CNFSM4HWUJB72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIFV4SI#issuecomment-571170377>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMK6SFSFHTBQEGRBKQ6NATQ4NBRDANCNFSM4HWUJB7Q> .

bors-servo · 2020-01-06T15:26:34Z

☀️ Test successful - status-taskcluster
State: approved= try=True

Manishearth · 2020-01-06T15:30:03Z

wow, rude, @bors-servo

Manishearth · 2020-01-06T15:31:50Z

@bors-servo r=jdm,manishearth try- retry

bors-servo · 2020-01-06T15:31:51Z

📌 Commit 508bfbd has been approved by jdm,manishearth

bors-servo · 2020-01-06T15:31:57Z

⌛ Testing commit 508bfbd with merge 5c7a4db...

Support type=module script element This is still WIP but hope can be reviewed first to see if I'm on the right track. Thanks! 🙇‍♂️ - [x] Support external module script - [x] Support internal module script - [x] Compile cyclic modules --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23370 (GitHub issue number if applicable) - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23545)

Manishearth · 2020-01-06T15:32:12Z

Color failures are #24726

bors-servo · 2020-01-06T15:33:00Z

☀️ Test successful - status-taskcluster
Approved by: jdm,manishearth
Pushing 5c7a4db to master...

I misunderstood the test case when I worked on servo#23545. That test case is actually not related to dynamic import; instead, the reason why it crashes is, `currentScript` should be updated to `null`.

Set `currentScript` to `null` for module scripts I misunderstood the test case when I worked on #23545. That test case is actually not related to dynamic import; instead, the reason why it crashes is, `currentScript` should be updated to `null`. In spec, the step 6 of [execute-the-script-block](https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-block) only says `Assert: document's currentScript attribute is null.` but doesn't says it should be set to null. Not sure if it can be improved. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable) - [x] There are tests for these changes

I misunderstood the test case when I worked on servo#23545. That test case is actually not related to dynamic import; instead, the reason why it crashes is, `currentScript` should be updated to `null`.

CYBAI requested review from jdm and nox June 10, 2019 14:58

jdm added the S-awaiting-review There is new code that needs to be reviewed. label Jun 10, 2019

highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 10, 2019

CYBAI force-pushed the support-module-script branch from e027b98 to 5d7de70 Compare June 10, 2019 15:28

highfive removed the S-tests-failed The changes caused existing tests to fail. label Jun 10, 2019

highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 10, 2019

KiChjang reviewed Jun 10, 2019

View reviewed changes

CYBAI force-pushed the support-module-script branch from 5d7de70 to 7371b4f Compare June 11, 2019 02:28

highfive removed the S-tests-failed The changes caused existing tests to fail. label Jun 11, 2019

highfive added the S-tests-failed The changes caused existing tests to fail. label Jun 11, 2019

highfive removed the S-tests-failed The changes caused existing tests to fail. label Jun 11, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 6, 2020

CYBAI mentioned this pull request Jan 6, 2020

Add a method to receive a vec of bits and return with SourceText servo/rust-mozjs#490

Closed

bors-servo merged commit 508bfbd into servo:master Jan 6, 2020

hello-webxr automation moved this from In progress to Done Jan 6, 2020

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 6, 2020

bors-servo mentioned this pull request Jan 6, 2020

Basic IndexedDB Functionality #25214

Open

4 tasks

CYBAI deleted the support-module-script branch January 6, 2020 15:38

jdm mentioned this pull request Jan 6, 2020

Support module worker #23308

Open

CYBAI mentioned this pull request Jan 6, 2020

Fail compilation for module script with UTF-16 BOM correctly #25441

Open

atouchet moved this from In progress to Done in A-Frame support Feb 23, 2020

atouchet moved this from In progress to Done in HoloLens Feb 23, 2020

CYBAI mentioned this pull request May 27, 2020

Set currentScript to null for module scripts #26668

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support type=module script element #23545

Support type=module script element #23545

CYBAI commented Jun 10, 2019 •

edited

CYBAI commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

CYBAI commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

KiChjang Jun 10, 2019

KiChjang Jun 10, 2019

KiChjang Jun 10, 2019

CYBAI commented Jun 11, 2019

bors-servo commented Jun 11, 2019

bors-servo commented Jun 11, 2019

jdm commented Jun 11, 2019

CYBAI commented Jun 11, 2019

CYBAI commented Jun 11, 2019

bors-servo commented Jun 11, 2019

bors-servo commented Jun 11, 2019

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

CYBAI commented Jan 6, 2020

Manishearth commented Jan 6, 2020 via email

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

Support type=module script element #23545

Support type=module script element #23545

Conversation

CYBAI commented Jun 10, 2019 • edited

CYBAI commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

CYBAI commented Jun 10, 2019

bors-servo commented Jun 10, 2019

bors-servo commented Jun 10, 2019

KiChjang Jun 10, 2019

Choose a reason for hiding this comment

KiChjang Jun 10, 2019

Choose a reason for hiding this comment

KiChjang Jun 10, 2019

Choose a reason for hiding this comment

CYBAI commented Jun 11, 2019

bors-servo commented Jun 11, 2019

bors-servo commented Jun 11, 2019

jdm commented Jun 11, 2019

CYBAI commented Jun 11, 2019

CYBAI commented Jun 11, 2019

bors-servo commented Jun 11, 2019

bors-servo commented Jun 11, 2019

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

CYBAI commented Jan 6, 2020

Manishearth commented Jan 6, 2020 via email

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

bors-servo commented Jan 6, 2020

Manishearth commented Jan 6, 2020

bors-servo commented Jan 6, 2020

CYBAI commented Jun 10, 2019 •

edited