Bug 132570 - [HTML] Default argument to HTMLTableSectionElement.insertRow() should be -1
Summary: [HTML] Default argument to HTMLTableSectionElement.insertRow() should be -1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: http://www.whatwg.org/specs/web-apps/...
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-05 09:49 PDT by Chris Dumez
Modified: 2014-05-06 10:55 PDT (History)
8 users (show)

See Also:


Attachments
Patch (6.41 KB, patch)
2014-05-05 10:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.61 KB, patch)
2014-05-06 10:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-05-05 09:49:57 PDT
The default argument to HTMLTableSectionElement.insertRow() should be -1, not 0, according to the specification:
http://www.whatwg.org/specs/web-apps/current-work/multipage/tabular-data.html#the-tbody-element 

Firefox 29 and IE11 both match the specification. Blink is about to change its behavior as well:
http://code.google.com/p/chromium/issues/detail?id=369803
https://codereview.chromium.org/263193002/
Comment 1 Chris Dumez 2014-05-05 10:25:59 PDT
Created attachment 230837 [details]
Patch
Comment 2 Chris Dumez 2014-05-05 15:35:10 PDT
Comment on attachment 230837 [details]
Patch

FYI, this change has now landed in Blink.
Comment 3 Chris Dumez 2014-05-06 06:13:56 PDT
FYI, if this change is approved, I will upload another one for HTMLTableRowElement.insertCell() as well (https://codereview.chromium.org/263323004/).
Comment 4 Darin Adler 2014-05-06 09:04:49 PDT
Comment on attachment 230837 [details]
Patch

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

> Source/WebCore/html/HTMLTableSectionElement.cpp:61
> +PassRefPtr<HTMLElement> HTMLTableSectionElement::insertRow(ExceptionCode& ec)
> +{
> +    // The default 'index' argument value is -1.
> +    return insertRow(-1, ec);
> +}

Why not put this into the header and mark it inline? I see no reason to add another level of function call overhead just to supply this default argument.
Comment 5 Chris Dumez 2014-05-06 10:14:45 PDT
Created attachment 230915 [details]
Patch
Comment 6 Chris Dumez 2014-05-06 10:15:48 PDT
Comment on attachment 230837 [details]
Patch

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

>> Source/WebCore/html/HTMLTableSectionElement.cpp:61
>> +}
> 
> Why not put this into the header and mark it inline? I see no reason to add another level of function call overhead just to supply this default argument.

Agreed, I updated the patch.
Comment 7 WebKit Commit Bot 2014-05-06 10:55:09 PDT
Comment on attachment 230915 [details]
Patch

Clearing flags on attachment: 230915

Committed r168365: <http://trac.webkit.org/changeset/168365>
Comment 8 WebKit Commit Bot 2014-05-06 10:55:14 PDT
All reviewed patches have been landed.  Closing bug.