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

Implement cells, rows, cellIndex IDL attributes and insertRow, deleteRow, insertCell, deleteCell, and cellIndex IDL methods for tabular data #6936

Closed
wants to merge 1 commit into from
Closed

Implement cells, rows, cellIndex IDL attributes and insertRow, deleteRow, insertCell, deleteCell, and cellIndex IDL methods for tabular data #6936

wants to merge 1 commit into from

Conversation

@dzbarsky
Copy link
Member

dzbarsky commented Aug 3, 2015

Review on Reviewable
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch 2 times, most recently from c48c3e2 to dc0289d Aug 3, 2015
@jdm jdm added the S-fails-tidy label Aug 4, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 4, 2015

Having this all squashed into one commit makes it hard to figure out if they all have sufficient tests.
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 4, 2015

The issue is that the existing WPT tests depends on all these features, so implementing them separately means none of the tests will pass. How about I make a list of spec sections/steps that this implements and for each one point out which test tests it (and add missing ones). That should make reviewing easier.
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 4, 2015

Hm, fun. That would probably help, thanks!
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch from dc0289d to 78ab0cb Aug 5, 2015
@jdm jdm removed the S-fails-tidy label Aug 6, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 7, 2015

Split the caption stuff into #7063
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch from 78ab0cb to beae219 Aug 7, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 8, 2015

Split createTBody into #7089
@nox
Copy link
Member

nox commented Aug 9, 2015

Should that one be closed?
@bors-servo
Copy link
Contributor

bors-servo commented Aug 9, 2015

☔ The latest upstream changes (presumably #7105) made this pull request unmergeable. Please resolve the merge conflicts.
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 9, 2015

@nox I'll try to split some other changes out, but some will stay in this PR.
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch 3 times, most recently from 060918a to e23bf12 Aug 9, 2015
@dzbarsky dzbarsky changed the title Implement some of the IDL attributes for tabular data [Do not review yet] Implement cells and rows attributes, insertRow/deleteRow, insertCell/deleteCell, and cellIndex IDL attributes/methods for tabular data Aug 9, 2015
@dzbarsky dzbarsky changed the title [Do not review yet] Implement cells and rows attributes, insertRow/deleteRow, insertCell/deleteCell, and cellIndex IDL attributes/methods for tabular data [Do not review yet] Implement cells, rows, cellIndex IDL attributes and insertRow, deleteRow, insertCell, deleteCell, and cellIndex IDL methods for tabular data Aug 9, 2015
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch 2 times, most recently from 382cdcc to 6d6607d Aug 9, 2015
@dzbarsky dzbarsky changed the title [Do not review yet] Implement cells, rows, cellIndex IDL attributes and insertRow, deleteRow, insertCell, deleteCell, and cellIndex IDL methods for tabular data Implement cells, rows, cellIndex IDL attributes and insertRow, deleteRow, insertCell, deleteCell, and cellIndex IDL methods for tabular data Aug 9, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 9, 2015

Ready for review. I tried to test that cells and rows filter out children elements that aren't of the right type, but I think the html5 parsing algorithm automatically wraps other elements in <td> or <tr><td> or something so I couldn't actually test that.
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 10, 2015

You may not be able to get there through the HTML parser, but script can certainly do it.
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch from 6d6607d to 759922b Aug 11, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Aug 11, 2015

Good point, I added tests for that.
@dzbarsky dzbarsky force-pushed the dzbarsky:table_idl branch from 759922b to 27d7cab Aug 14, 2015
@dzbarsky dzbarsky mentioned this pull request Sep 27, 2015
Merged
@dzbarsky
Copy link
Member Author

dzbarsky commented Sep 28, 2015

@Ms2ger review ping?
@frewsxcv
Copy link
Member

frewsxcv commented Sep 28, 2015

As per this comment, unless @dzbarsky says otherwise I can take ownership of this PR.
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 2, 2015
@dzbarsky @frewsxcv
Implement HTMLTableRowElement::Cells
4e64ff4 
Extracted from servo#6936
@frewsxcv frewsxcv mentioned this pull request Oct 2, 2015
Merged
@frewsxcv
Copy link
Member

frewsxcv commented Oct 2, 2015

Going to open these into smaller individual PRs (starting with #7823). No need to keep this open.
@frewsxcv frewsxcv closed this Oct 2, 2015
bors-servo pushed a commit that referenced this pull request Oct 2, 2015
bors-servo
Auto merge of #7823 - frewsxcv:htmltablerowelement-cells, r=Ms2ger
Loading status checks…
8865410 
Implement HTMLTableRowElement::Cells

Extracted from #6936

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7823)
<!-- Reviewable:end -->
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 2, 2015
@dzbarsky @frewsxcv
Implement HTMLTableCellElement::CellIndex
82d7cfa 
Extracted from servo#6936
@frewsxcv frewsxcv mentioned this pull request Oct 2, 2015
Merged
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 2, 2015
@dzbarsky @frewsxcv
Implement HTMLTableCellElement::CellIndex
e7b869f 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 3, 2015
@dzbarsky @frewsxcv
Implement HTMLTableCellElement::CellIndex
68bdef5 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 3, 2015
@frewsxcv
Implement HTMLTableCellElement::CellIndex
8d4ce78 
Extracted from servo#6936
components/script/dom/webidls/HTMLTableElement.webidl
@@ -6,7 +6,7 @@
// https://www.whatwg.org/html/#htmltableelement
interface HTMLTableElement : HTMLElement {
attribute HTMLTableCaptionElement? caption;
HTMLElement createCaption();

This comment has been minimized.

Sign in to view
@frewsxcv

frewsxcv Oct 3, 2015

Member

Do you remember why you did this @dzbarsky? For reference: https://html.spec.whatwg.org/multipage/tables.html#htmltableelement

This comment has been minimized.

Sign in to view
@dzbarsky

dzbarsky Oct 5, 2015

Author Member

It lets us get rid of the HTMLElementCasts, and it doesn't really matter what type you declare as being returned in the IDL, what matters is the properties/methods on the type actually returned.

This comment has been minimized.

Sign in to view
@dzbarsky

dzbarsky Oct 5, 2015

Author Member

We can probably file a spec bug for this if we care

This comment has been minimized.

Sign in to view
@nox

nox Oct 5, 2015

Member

I would rather keep the casts for now.
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 4, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
7afa3ba 
Continued from servo#6936
@frewsxcv frewsxcv mentioned this pull request Oct 4, 2015
Merged
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 4, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
1eb0360 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 4, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
2e2e972 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 6, 2015
@frewsxcv
Implement HTMLTableCellElement::CellIndex
c252fc5 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 6, 2015
@frewsxcv
Implement HTMLTableCellElement::CellIndex
d335e85 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 6, 2015
@frewsxcv
Implement HTMLTableCellElement::CellIndex
4df2de4 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 6, 2015
@frewsxcv
Implement HTMLTableCellElement::CellIndex
899f1ca 
Extracted from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 8, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
d439c8b 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 8, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
ade92b5 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 8, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
56a919d 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 9, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
5adb835 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 9, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
7a03666 
Continued from servo#6936
frewsxcv added a commit to frewsxcv/servo that referenced this pull request Oct 11, 2015
@frewsxcv
Implement deleteRow and insertRow for <table> element
3d383f2 
Continued from servo#6936
bors-servo pushed a commit that referenced this pull request Oct 11, 2015
bors-servo
Auto merge of #7854 - frewsxcv:htmltablesectionelement-insertrow-dele…
Loading status checks…
292dbfe 
…terow, r=nox

Implement deleteRow and insertRow for <table> element

Continued from #6936

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7854)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this pull request Oct 14, 2015
bors-servo
Auto merge of #7829 - frewsxcv:htmltablecellelement-cellindex, r=Ms2ger
Loading status checks…
55769b2 
Implement HTMLTableCellElement::CellIndex

Extracted from #6936

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7829)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
S-awaiting-review S-needs-rebase
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@dzbarsky @Ms2ger @nox @bors-servo @frewsxcv @jdm @highfive
You can’t perform that action at this time.