Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly insert text in document when parsing #14867

Merged
merged 3 commits into from Jan 7, 2017
Merged

Properly insert text in document when parsing #14867

merged 3 commits into from Jan 7, 2017

Conversation

@nox
Copy link
Member

nox commented Jan 5, 2017 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Jan 5, 2017

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/dom/htmlstyleelement.rs, components/script/Cargo.toml, components/script/dom/servoparser/html.rs, components/script/dom/virtualmethods.rs
  • @KiChjang: components/script/dom/htmlstyleelement.rs, components/script/Cargo.toml, components/script/dom/servoparser/html.rs, components/script/dom/virtualmethods.rs
@highfive
Copy link

highfive commented Jan 5, 2017

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@nox
Copy link
Member Author

nox commented Jan 5, 2017

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2017

⌛ Trying commit c54eea7 with merge 5906442...
bors-servo added a commit that referenced this pull request Jan 5, 2017
@bors-servo
Auto merge of #14867 - nox:a-base-de-popopopop, r=<try>
Loading status checks…
5906442 
Update html5ever to 0.12.1 and make use of its pop hook
@metajack
Copy link
Contributor

metajack commented Jan 5, 2017

This needs a little bit of comment. It's not clear what this is for unless you read the spec.

r=me with that addressed.
@nox nox force-pushed the nox:a-base-de-popopopop branch from c54eea7 to c8f951b Jan 5, 2017
@nox
Copy link
Member Author

nox commented Jan 5, 2017

@bors-servo r=jack
@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2017

📌 Commit c8f951b has been approved by jack
@jdm jdm mentioned this pull request Jan 5, 2017
Closed
1 of 5 tasks complete
@nox nox force-pushed the nox:a-base-de-popopopop branch from c8f951b to cf93159 Jan 6, 2017
@nox
Copy link
Member Author

nox commented Jan 6, 2017

@bors-servo r=jack
@bors-servo
Copy link
Contributor

bors-servo commented Jan 6, 2017

📌 Commit cf93159 has been approved by jack
@bors-servo
Copy link
Contributor

bors-servo commented Jan 6, 2017

⌛ Testing commit cf93159 with merge f976cac...
bors-servo added a commit that referenced this pull request Jan 6, 2017
@bors-servo
Auto merge of #14867 - nox:a-base-de-popopopop, r=jack
Loading status checks…
f976cac 
Update html5ever to 0.12.1 and make use of its pop hook

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14867)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 6, 2017

💔 Test failed - linux-rel-wpt
@nox
Copy link
Member Author

nox commented Jan 6, 2017

I forgot to update x5e, so style elements don't get parsed at all when in XHTML now...

Looking into the other failures now.
@nox
Copy link
Member Author

nox commented Jan 6, 2017

Oh nice. The new line number tracking means that every "\n" gets its own Text node because we don't coalesce adjacent Text children when parsing, thus many unrelated test failures.
@nox
Copy link
Member Author

nox commented Jan 6, 2017

https://github.com/Ygg01/xml5ever/pull/24 for the XML issues.
@highfive highfive removed the S-tests-failed label Jan 6, 2017
@nox
Copy link
Member Author

nox commented Jan 6, 2017

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 6, 2017

⌛ Trying commit 30fce30 with merge 4e2318e...
bors-servo added a commit that referenced this pull request Jan 6, 2017
@bors-servo
Auto merge of #14867 - nox:a-base-de-popopopop, r=<try>
Loading status checks…
4e2318e 
Properly insert text in document when parsing

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14867)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 6, 2017

💔 Test failed - linux-rel-wpt
@nox nox force-pushed the nox:a-base-de-popopopop branch from 30fce30 to 764ee1c Jan 7, 2017
@highfive highfive removed the S-tests-failed label Jan 7, 2017
@nox
Copy link
Member Author

nox commented Jan 7, 2017

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2017

⌛ Trying commit 764ee1c with merge 1eb1ea0...
bors-servo added a commit that referenced this pull request Jan 7, 2017
@bors-servo
Auto merge of #14867 - nox:a-base-de-popopopop, r=<try>
Loading status checks…
1eb1ea0 
Properly insert text in document when parsing

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14867)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2017

💔 Test failed - linux-rel-wpt
@nox nox force-pushed the nox:a-base-de-popopopop branch from 764ee1c to e8ed19b Jan 7, 2017
@highfive highfive removed the S-tests-failed label Jan 7, 2017
nox added 3 commits Jan 6, 2017
@nox
Update html5ever to 0.12.1
a89bbc9
@nox
Update xml5ever to 0.3.0
8caa564
@nox
Properly insert text in document when parsing
Loading status checks…
0cb0946 
If the last child of a node is a Text child and we are inserting text in that node,
we need to append it to that Text child. Doing that means that
HTMLStyleElement::children_changed gets called less frequently, but that's not a
problem during parsing thanks to the pop hook.
@nox nox force-pushed the nox:a-base-de-popopopop branch from e8ed19b to 0cb0946 Jan 7, 2017
@nox
Copy link
Member Author

nox commented Jan 7, 2017

@bors-servo r=metajack
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2017

📌 Commit 0cb0946 has been approved by metajack
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2017

⌛ Testing commit 0cb0946 with merge f313e8b...
bors-servo added a commit that referenced this pull request Jan 7, 2017
@bors-servo
Auto merge of #14867 - nox:a-base-de-popopopop, r=metajack
Loading status checks…
f313e8b 
Properly insert text in document when parsing

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14867)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
@bors-servo bors-servo merged commit 0cb0946 into servo:master Jan 7, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Jan 7, 2017
Merged
3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@metajack metajack

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@nox @highfive @bors-servo @metajack
You can’t perform that action at this time.