splitText API does not match as per DOM specification, as per latest Spec: https://w3c.github.io/dom/#interface-text, change splitText() argument to mandatory.
Created attachment 241013 [details] Patch splitText API does not match as per DOM specification, modifed as per spec and added new tests
Created attachment 241019 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 241021 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 241013 [details] Patch The test fast/dom/non-numeric-values-numeric-parameters.html, or at least its expected results, has to be changed, too.
Created attachment 241083 [details] Patch-Updated Patch updated with test failures.
(In reply to comment #4) > Comment on attachment 241013 [details] > Patch > > The test fast/dom/non-numeric-values-numeric-parameters.html, or at least > its expected results, has to be changed, too. Dear Darin, Thanks, updated the patch with test results.
Comment on attachment 241083 [details] Patch-Updated View in context: https://bugs.webkit.org/attachment.cgi?id=241083&action=review The web-exposed behavior change seems fine as it matches the specification and the behavior other major browsers but please take into consideration my comments. > Source/WebCore/ChangeLog:7 > + This matches the behavior of both Firefox 33 and Chrome 38. Please state so in your Changelog as this is more important than matching the specification. Also, please indicate in the Changelog which web-exposed changes you are making: 1. Make the offset argument mandatory (and thus throw if it is omitted) 2. No longer throw if the offset argument is negative (and wraps to a valid index) > LayoutTests/fast/dom/Text/splitText.html:14 > +shouldThrow('text.splitText()'); Please specify the second argument to shouldThrow (exception name) > LayoutTests/fast/dom/Text/splitText.html:15 > +shouldThrow('text.splitText(999)'); ditto. > LayoutTests/fast/dom/Text/splitText.html:16 > +shouldThrow('text.splitText(-1)'); ditto.
Created attachment 241085 [details] Patch-Updated-Review2 Updated change log and splitText.html test file sa per comments.
Comment on attachment 241085 [details] Patch-Updated-Review2 r=me, thanks.
Comment on attachment 241085 [details] Patch-Updated-Review2 Clearing flags on attachment: 241085 Committed r175688: <http://trac.webkit.org/changeset/175688>
All reviewed patches have been landed. Closing bug.
Comment on attachment 241085 [details] Patch-Updated-Review2 View in context: https://bugs.webkit.org/attachment.cgi?id=241085&action=review I'll fix these nits in a follow-up patch as I missed them during my earlier review. > LayoutTests/fast/dom/Text/splitText.html:1 > +<html> Missing Doctype. > LayoutTests/fast/dom/Text/splitText.html:5 > +<head> 2 heads?
(In reply to comment #12) > Comment on attachment 241085 [details] > Patch-Updated-Review2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241085&action=review > > I'll fix these nits in a follow-up patch as I missed them during my earlier > review. > > > LayoutTests/fast/dom/Text/splitText.html:1 > > +<html> > > Missing Doctype. > > > LayoutTests/fast/dom/Text/splitText.html:5 > > +<head> > > 2 heads? Fixed nits in <http://trac.webkit.org/changeset/175689>.
(In reply to comment #13) > (In reply to comment #12) > > Comment on attachment 241085 [details] > > Patch-Updated-Review2 > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=241085&action=review > > > > I'll fix these nits in a follow-up patch as I missed them during my earlier > > review. > > > > > LayoutTests/fast/dom/Text/splitText.html:1 > > > +<html> > > > > Missing Doctype. > > > > > LayoutTests/fast/dom/Text/splitText.html:5 > > > +<head> > > > > 2 heads? > > Fixed nits in <http://trac.webkit.org/changeset/175689>. Dear Chris, Thanks, for adding patch, which i missed.