Skip to content
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

Implement createCaption and deleteCaption on HTMLTableElement #7063

Merged
merged 1 commit into from Aug 9, 2015

Conversation

@dzbarsky
Copy link
Member

dzbarsky commented Aug 7, 2015

Review on Reviewable

@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 7, 2015

Make tests/wpt/web-platform-tests/html/semantics/tabular-data/the-caption-element/caption_001.html test that the new caption is indeed the first child.


Reviewed 7 of 7 files at r1.
Review status: all files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.


components/script/dom/htmltableelement.rs, line 88 [r1] (raw file):
Maybe a little too clever; feel free to use a local variable for the first child.


tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/caption-methods.html, line 15 [r1] (raw file):
This is HTML, so foo:caption isn't a particularly interesting case. You'll need some dynamically generated tables.


tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/caption-methods.html, line 54 [r1] (raw file):
And assert_equals(testCaption, table0.firstChilfd)


tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/caption-methods.html, line 84 [r1] (raw file):
Come again?


tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/caption-methods.html, line 86 [r1] (raw file):
For bonus points, add a (dynamic) test for createCaption() where the table element has a prefix; the newly created caption should not have a prefix. (Gecko should fail this, I believe.)


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 7, 2015

-S-awaiting-review +S-needs-code-changes


Review status: all files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 8, 2015

Squash?

-S-awaiting-review +S-needs-squash


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


Comments from the review on Reviewable.io

@dzbarsky dzbarsky force-pushed the dzbarsky:caption branch from dea05ae to e24a867 Aug 8, 2015
Update web-platform-tests expected data
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 9, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2015

📌 Commit e24a867 has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2015

Testing commit e24a867 with merge f41834e...

bors-servo pushed a commit that referenced this pull request Aug 9, 2015
Implement createCaption and deleteCaption on HTMLTableElement



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7063)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2015

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2, mac3

@bors-servo bors-servo merged commit e24a867 into servo:master Aug 9, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.