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.

Sign up for GitHub

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

Jump to bottom

Merge the two table cell interfaces back together #1125

Merged
merged 1 commit into from
Apr 26, 2016
Merged

Merge the two table cell interfaces back together #1125

merged 1 commit into from
Apr 26, 2016

Conversation

domenic
Copy link
Member

@domenic domenic commented Apr 25, 2016

Ever since table sorting was introduced, HTMLTableCellElement was split
into two descendant interfaces, HTMLTableDataCellElement for <td>, and
HTMLTableHeaderCellElement for <th>. No browser ever implemented this
split. Although table sorting was removed in
59b7e24,
the split remained; we now remove it, fixing #1115.

@domenic domenic added the removal/deprecation Removing or deprecating a feature label Apr 25, 2016
@annevk
Copy link
Member

annevk commented Apr 26, 2016

Did table sorting introduce the split? It seems to me it was introduced earlier, to deal with the semantic differences.

annevk
annevk reviewed Apr 26, 2016
View reviewed changes
source
readonly attribute long <span data-x="dom-tdth-cellIndex">cellIndex</span>;

[<span>CEReactions</span>] attribute DOMString <span data-x="dom-th-scope">scope</span>; // only used for th elements
[<span>CEReactions</span>] attribute DOMString <span data-x="dom-th-abbr">abbr</span>; // only used for th elements
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should say "only conforming" since they'll work either way. @zcorpan might want to take a look too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with "only conforming"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure conforming was relevant to IDL attributes but works for me

@domenic
Copy link
Member Author

domenic commented Apr 26, 2016

Did table sorting introduce the split? It seems to me it was introduced earlier, to deal with the semantic differences.

I should probably check with history but my recollection is that it was introduced so that you could add table sorting-specific methods and attributes to <th> elements.

@annevk
Copy link
Member

annevk commented Apr 26, 2016 •
edited
Loading

Nope, see 6db0d8d (modifies HTMLTableHeaderCellElement, doesn't introduce it).

@annevk
Copy link
Member

annevk commented Apr 26, 2016

7424da0 has something to do with it, but it seems it started before that and I can't find when.

@annevk
Copy link
Member

annevk commented Apr 26, 2016

Based on 2ac31bb and 7ccc7fc it has always been this way.

And it is better design, but it was just never implemented or cared for.

@domenic
Merge the two table cell interfaces together
e383ae2 
Ever since the dawn of HTML (2ac31bb),
there have been two table cell interfaces, HTMLTableDataCellElement for
<td>, and HTMLTableHeaderCellElement for <th>. No browser ever
implemented this split, instead using the single HTMLTableCellElement
interface. This aligns the spec with browser reality.

Fixes #1115.
@domenic
Copy link
Member Author

domenic commented Apr 26, 2016

Updated commit message and changed to "only conforming"; please take a look.

@annevk
Copy link
Member

annevk commented Apr 26, 2016

Nice, LGTM.

@domenic domenic merged commit e383ae2 into master Apr 26, 2016
@domenic domenic deleted the merge-table-interfaces branch April 26, 2016 17:11
@domenic domenic mentioned this pull request Jun 9, 2016
Merged
@cbrewster cbrewster mentioned this pull request Jun 8, 2017
Closed
@keithamus keithamus mentioned this pull request Dec 4, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
removal/deprecation Removing or deprecating a feature
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@domenic @annevk @zcorpan