Summary: | [HTML] Default argument to HTMLTableSectionElement.insertRow() should be -1 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | DOM | Assignee: | 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
Chris Dumez
2014-05-05 09:49:57 PDT
Created attachment 230837 [details]
Patch
Comment on attachment 230837 [details]
Patch
FYI, this change has now landed in Blink.
FYI, if this change is approved, I will upload another one for HTMLTableRowElement.insertCell() as well (https://codereview.chromium.org/263323004/). 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. Created attachment 230915 [details]
Patch
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 on attachment 230915 [details] Patch Clearing flags on attachment: 230915 Committed r168365: <http://trac.webkit.org/changeset/168365> All reviewed patches have been landed. Closing bug. |