Bug 132570

Summary: [HTML] Default argument to HTMLTableSectionElement.insertRow() should be -1
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, darin, eric, esprehn+autocc, gyuyoung.kim, kondapallykalyan, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.whatwg.org/specs/web-apps/current-work/multipage/tabular-data.html#the-tbody-element
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 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/
Attachments
Patch (6.41 KB, patch)
2014-05-05 10:25 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (5.61 KB, patch)
2014-05-06 10:14 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2014-05-05 10:25:59 PDT
Created attachment 230837 [details] Patch
Chris Dumez
Comment 2 2014-05-05 15:35:10 PDT
Comment on attachment 230837 [details] Patch FYI, this change has now landed in Blink.
Chris Dumez
Comment 3 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/).
Darin Adler
Comment 4 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.
Chris Dumez
Comment 5 2014-05-06 10:14:45 PDT
Created attachment 230915 [details] Patch
Chris Dumez
Comment 6 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.
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2014-05-06 10:55:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.