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

HTMLTableElement's rows order is not tested in wpt #4470

Closed
domenic opened this issue Mar 29, 2019 · 8 comments
Closed

HTMLTableElement's rows order is not tested in wpt #4470

domenic opened this issue Mar 29, 2019 · 8 comments
Labels
needs tests Moving the issue forward requires someone to write tests topic: table

Comments

@domenic
Copy link
Member

domenic commented Mar 29, 2019

Porting from https://www.w3.org/Bugs/Public/show_bug.cgi?id=29018.

The order of rows in that collection is apparently not interoperable between browsers. There are competing proposals, with @foolip and @zcorpan proposing (in https://www.w3.org/Bugs/Public/show_bug.cgi?id=29018#c7):

something that matches the prose for table-header-group and table-footer-group in http://www.w3.org/TR/CSS2/tables.html#table-display

and @annevk proposing (https://www.w3.org/Bugs/Public/show_bug.cgi?id=29018#c16):

FWIW, I think what the standard says today here makes the most sense. We should simply change Firefox and Edge.

For the record, as of today the standard says

The elements in the collection must be ordered such that those elements whose parent is a thead are included first, in tree order, followed by those elements whose parent is either a table or tbody element, again in tree order, followed finally by those elements whose parent is a tfoot element, still in tree order.

I tend to agree the standard sounds pretty nice and simple, and we should consider just writing tests and filing a Firefox bug, with no spec changes.

@domenic domenic added interop Implementations are not interoperable with each other topic: table labels Mar 29, 2019
@zcorpan
Copy link
Member

zcorpan commented Mar 29, 2019

I believe @annevk opinion was based on misunderstanding my and @foolip's proposal (that it would be based on the rendering).

The proposal is:

  • Move the first thead so that its rows are first in the list.
  • Move the first tfoot so that its rows are last in the list.
  • Don't move any other rows.

This matches default rendering, and is strictly less moving of rows than what the spec does today.

@domenic
Copy link
Member Author

domenic commented Mar 29, 2019

And I guess Blink and WebKit match the current spec, whereas Gecko matches your proposal? But, we want to write web platform tests to confirm?

@zcorpan
Copy link
Member

zcorpan commented Mar 29, 2019

Looking at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3778 in Firefox Nightly it seems it matches the current spec now also.

@zcorpan zcorpan added needs tests Moving the issue forward requires someone to write tests and removed interop Implementations are not interoperable with each other labels Mar 29, 2019
@zcorpan zcorpan changed the title HTMLTableElement's rows order is not interoperable HTMLTableElement's rows order is not tested in wpt Mar 29, 2019
@muodov
Copy link
Contributor

muodov commented Oct 4, 2020

I'd like to work on this. If I understand correctly, there is no compatibility problems between the major browsers and the spec, and we just need the tests?
Should I create a corresponding issue in the web-platform-tests repo, or just file a PR right away?

@annevk
Copy link
Member

annevk commented Oct 5, 2020

That's great! And yeah, I think that's correct. web-platform-tests uses issues for coordination or backlog. If you can address something immediately (i.e., via a PR) there's no need for an issue.

@muodov
Copy link
Contributor

muodov commented Oct 7, 2020

Hm, it seems like there is already a test for this. The "complicated case" test case asserts the results of the following HTML, which seems to cover the cases discussed here. E.g. it is asserted that all tfoot rows, including the first one, are returned after all the thead and tbody rows, just like the spec says.

Asserted HTML
<table>
    <tr id="orphan1"></tr>
    <tfoot>
        <tr id="foot1row1"></tr>
        <tr id="foot1row2"></tr>
        <div>
            <tr></tr>
        </div>
        <tr></tr>
    </tfoot>
    <tr id="orphan2"></tr>
    <tfoot>
        <tr id="foot2row1"></tr>
        <tr id="foot2row2"></tr>
    </tfoot>
    <tr id="orphan3"></tr>
    <tbody>
        <tr id="body1row1"></tr>
        <tr id="body1row2"></tr>
        <div>
            <tr></tr>
        </div>
        <tr></tr>
    </tbody>
    <tr id="orphan4"></tr>
    <tbody>
        <tr id="body2row1"></tr>
        <tr id="body2row2"></tr>
    </tbody>
    <tr id="orphan5"></tr>
    <thead>
        <tr id="head1row1"></tr>
        <tr id="head1row2"></tr>
        <div>
            <tr></tr>
        </div>
        <tr></tr>
    </thead>
    <tr id="orphan6"></tr>
    <thead>
        <tr id="head2row1"></tr>
        <tr id="head2row2"></tr>
    </thead>
    <tr id="orphan7"></tr>
    <div>
        <tr></tr>
    </div>
    <tr></tr>
</table>

This test is passing in all browsers monitored by wpt.fyi. Shall we just close this ticket? :)

@annevk
Copy link
Member

annevk commented Oct 8, 2020

Thanks, it seems that test was made as part of fixing the bug in Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1264947. So yeah, everything is in order!

@annevk annevk closed this as completed Oct 8, 2020
@domenic
Copy link
Member Author

domenic commented Oct 8, 2020

Thank you @muodov for your willingness to dive in and help us close this out!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Moving the issue forward requires someone to write tests topic: table
Development

No branches or pull requests

4 participants