Implement the create an element for token algorithm #19397

cbrewster · 2017-11-27T19:58:36Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #19392 and fix #19393 (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2017-11-27T19:58:40Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/servoparser/mod.rs, components/script/dom/servoparser/async_html.rs
@fitzgen: components/script/dom/servoparser/mod.rs, components/script/dom/servoparser/async_html.rs
@KiChjang: components/script/dom/servoparser/mod.rs, components/script/dom/servoparser/async_html.rs

highfive · 2017-11-27T19:58:41Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

cbrewster · 2017-11-27T19:59:33Z

r? @jdm
Was there a specific test case that I could add here?
Also do we have a way of determining if the JavaScript execution context stack is empty?

jdm · 2017-11-27T20:38:53Z

The test that led me to discover this was to take parser-fallsback-to-unknown-element.html and add a document.write() call in an onerror handler. Without #19393 the parser is in an unexpected state and we get a borrow panic.

jdm · 2017-11-27T20:49:50Z

components/script/dom/servoparser/mod.rs

+        // Step 6.1.
+        // TODO: handle throw-on-dynamic-markup-insertion counter.
+        // Step 6.2
+        // TODO: If the JavaScript execution context stack is empty, then perform a microtask checkpoint.


We should add an API to https://github.com/servo/servo/blob/master/components/script/dom/bindings/settings_stack.rs to check this.

jdm · 2017-11-27T20:49:50Z

components/script/dom/servoparser/mod.rs

+
+    // Step 5.
+    // TODO: Check if the parser was created for the HTML fragment parsing algorithm
+    let will_execute_scrtipt = definition.is_some();


jdm · 2017-11-27T20:49:50Z

components/script/dom/servoparser/mod.rs

+    let definition = document.lookup_custom_element_definition(&name.ns, &name.local, is.as_ref());
+
+    // Step 5.
+    // TODO: Check if the parser was created for the HTML fragment parsing algorithm


We should add a field to ServoParser with this information.

jdm · 2017-11-27T20:49:50Z

components/script/dom/servoparser/mod.rs

+
+    // TODO: Step 10.
+    // TODO: Step 11.
+    // TODO: Step 12.


Step 12 is handled from the parser callback associate_with_form.

jdm · 2018-01-04T18:09:16Z

@bors-servo: try

bors-servo · 2018-01-04T18:09:21Z

⌛ Trying commit df301b5 with merge 5f72bae...


        Auto merge of #19397 - cbrewster:create_element_for_token, r=<try>

Implement the create an element for token algorithm  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19392 (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/19397)

bors-servo · 2018-01-04T18:09:26Z

💔 Test failed - linux-rel-wpt

jdm · 2018-01-04T18:09:42Z

Bleah:

./components/script/dom/servoparser/mod.rs:110: derivable traits list is not in alphabetical order
	expected: Clone, Copy, JSTraceable, MallocSizeOf, PartialEq
	found: Clone, Copy, PartialEq, JSTraceable, MallocSizeOf

jdm · 2018-01-08T23:19:11Z

@bors-servo try

bors-servo · 2018-01-08T23:19:16Z

⌛ Trying commit f7e8f56 with merge 3fc40f7...


        Auto merge of #19397 - cbrewster:create_element_for_token, r=<try>

Implement the create an element for token algorithm  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19392 and fix #19393 (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/19397)

bors-servo · 2018-01-09T00:00:29Z

💔 Test failed - mac-rel-wpt1

CYBAI · 2018-01-09T02:07:36Z

Failed with #19495

KiChjang · 2018-01-09T13:38:06Z

@bors-servo retry

bors-servo · 2018-01-09T13:38:10Z

⌛ Trying commit f7e8f56 with merge 141192d...


        Auto merge of #19397 - cbrewster:create_element_for_token, r=<try>

Implement the create an element for token algorithm  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19392 and fix #19393 (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/19397)

jdm · 2018-01-09T13:56:55Z

I appreciate the support, but I was watching the build and it didn't need a retry.

bors-servo · 2018-01-09T15:08:56Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True


        Implement the create an element for token algorithm


        Add a check for when the js execution stack is empty


        Implement `throw-on-dynamic-markup-insertion-counter`


        Take throw-on-dynamic-markup-insertion-counter into account when chec…

…k script safety.

jdm · 2018-01-11T16:15:18Z

r? @nox for the last commit. I've reviewed the rest of them.

nox · 2018-01-17T16:22:57Z

@bors-servo r=jdm,nox

bors-servo · 2018-01-17T16:22:58Z

📌 Commit c55f25b has been approved by jdm,nox

bors-servo · 2018-01-17T16:57:08Z

⌛ Testing commit c55f25b with merge fa82a6b...


        Auto merge of #19397 - cbrewster:create_element_for_token, r=jdm,nox

Implement the create an element for token algorithm  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19392 and fix #19393 (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/19397)

bors-servo · 2018-01-17T18:12:14Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm,nox
Pushing fa82a6b to master...

highfive assigned KiChjang Nov 27, 2017

highfive added the S-awaiting-review label Nov 27, 2017

highfive assigned jdm and unassigned KiChjang Nov 27, 2017

jdm requested changes Nov 27, 2017

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Nov 27, 2017

cbrewster force-pushed the cbrewster:create_element_for_token branch from e17b271 to df301b5 Dec 3, 2017

highfive added S-awaiting-review and removed S-needs-code-changes labels Dec 3, 2017

highfive added the S-tests-failed label Jan 4, 2018

jdm force-pushed the cbrewster:create_element_for_token branch from df301b5 to f7e8f56 Jan 8, 2018

highfive removed the S-tests-failed label Jan 8, 2018

highfive added the S-tests-failed label Jan 9, 2018

highfive removed the S-tests-failed label Jan 11, 2018

jdm force-pushed the cbrewster:create_element_for_token branch from 280a133 to 10010ce Jan 11, 2018

cbrewster and others added 4 commits Nov 27, 2017

Implement the create an element for token algorithm

87475b1

Add a check for when the js execution stack is empty

43526c8

Implement `throw-on-dynamic-markup-insertion-counter`

4593195

Take throw-on-dynamic-markup-insertion-counter into account when chec…

Loading status checks…

c55f25b

…k script safety.

jdm force-pushed the cbrewster:create_element_for_token branch from 10010ce to c55f25b Jan 11, 2018

highfive assigned nox and unassigned jdm Jan 11, 2018

highfive assigned jdm and unassigned nox Jan 17, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 17, 2018

bors-servo merged commit c55f25b into servo:master Jan 17, 2018
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jan 17, 2018

servo / servo

Implement the create an element for token algorithm #19397

Implement the create an element for token algorithm #19397

cbrewster commented Nov 27, 2017 •

edited by jdm

highfive commented Nov 27, 2017

highfive commented Nov 27, 2017

cbrewster commented Nov 27, 2017

jdm commented Nov 27, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented Jan 4, 2018

bors-servo commented Jan 4, 2018

bors-servo commented Jan 4, 2018

jdm commented Jan 4, 2018

jdm commented Jan 8, 2018

bors-servo commented Jan 8, 2018

bors-servo commented Jan 9, 2018

CYBAI commented Jan 9, 2018

KiChjang commented Jan 9, 2018

bors-servo commented Jan 9, 2018

jdm commented Jan 9, 2018

bors-servo commented Jan 9, 2018

jdm commented Jan 11, 2018

nox commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

servo / servo

Join GitHub today

Implement the create an element for token algorithm #19397

Implement the create an element for token algorithm #19397

Conversation

cbrewster commented Nov 27, 2017 • edited by jdm

highfive commented Nov 27, 2017

highfive commented Nov 27, 2017

cbrewster commented Nov 27, 2017

jdm commented Nov 27, 2017

This comment has been minimized.

jdm Nov 27, 2017

This comment has been minimized.

jdm Nov 27, 2017

This comment has been minimized.

jdm Nov 27, 2017

This comment has been minimized.

jdm Nov 27, 2017

jdm commented Jan 4, 2018

bors-servo commented Jan 4, 2018

bors-servo commented Jan 4, 2018

jdm commented Jan 4, 2018

jdm commented Jan 8, 2018

bors-servo commented Jan 8, 2018

bors-servo commented Jan 9, 2018

CYBAI commented Jan 9, 2018

KiChjang commented Jan 9, 2018

bors-servo commented Jan 9, 2018

jdm commented Jan 9, 2018

bors-servo commented Jan 9, 2018

jdm commented Jan 11, 2018

nox commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

cbrewster commented Nov 27, 2017 •

edited by jdm