Bug 93738 – table cell.cellIndex should return -1 if there is no parent table

• Component: DOM
• Status: RESOLVED FIXED

[Ms2ger (he/him; ⌚ UTC+1/+2)]

See <http://w3c-test.org/html/tests/submission/Ms2ger/the-elements-of-html/tabular-data/attributes-common-to-td-and-th-elements/cellIndex.html>; spec was changed in <http://html5.org/tools/web-apps-tracker?from=7242&to=7243>.

[Pravin D]

Created attachment 157874 [details]
Patch

[Pravin D]

A little off the topic... We have functions
    // FIXME: need to implement cellIndex
    int cellIndex() const { return 0; }
    void setCellIndex(int) { }

in RenderTableCell.h ... Is this still required ? It does seem to be used anywhere...

It was added in 
http://trac.webkit.org/browser/trunk/WebCore/khtml/dom/html_table.h?annotate=blame&rev=4

[Eric Seidel (no email)]

Comment on attachment 157874 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157874&action=review

It also appears we have a RenderTableCell::cellIndex() function which does nothing and probably shoudl be removed.

> Source/WebCore/html/HTMLTableCellElement.cpp:73
> +    int index = -1;
> +    if (!parentElement() || !parentElement()->hasTagName(trTag))
> +        return index;
> +
> +    index = 0;

I would have jsut used "return -1" here, and left int index = 0; as it was.  It saves you a line of typing.

[Eric Seidel (no email)]

Comment on attachment 157874 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157874&action=review

> LayoutTests/fast/table/cellIndex-of-cell-with-different-parents.html:33
> +shouldBeTrue('th_with_tbody_parent.cellIndex == -1');

This could read shouldBe("th_with_tbody_parent.cellIndex", "-1");

[Pravin D]

Created attachment 157894 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 157894 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157894&action=review

Thanks!

> Source/WebCore/html/HTMLTableCellElement.cpp:69
>      int index = 0;

We don't do K&R style anymore.  This declaration could be inline, right before the for() below, but the patch overall is fine, and clearly the rest of this file needs a style cleanup some day anyway.

[WebKit Review Bot]

Comment on attachment 157894 [details]
Patch

Clearing flags on attachment: 157894

Committed r125380: <http://trac.webkit.org/changeset/125380>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.