Remove custom bindings for HTMLInputElement
Created attachment 54732 [details] Patch
Attachment 54732 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:440: Extra space before ( in function call [whitespace/parens] [4] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:456: Extra space before ( in function call [whitespace/parens] [4] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:481: Extra space before ( in function call [whitespace/parens] [4] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:591: Extra space before ) [whitespace/parens] [2] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:597: Extra space before ) [whitespace/parens] [2] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:164: Extra space before ( in function call [whitespace/parens] [4] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:167: Extra space before ( in function call [whitespace/parens] [4] WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:173: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 8 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
That's better.
Comment on attachment 54732 [details] Patch I think HTMLInputElement.h needs a comment as to why we're adding these. But I think this looks OK. Should we be be making selectionStart, etc. virtual and overriding them in HTMLIntputElement as private members? Should we change the baseclass to take an ExceptionCode and only throw it for HTMLInputElement? I think this is OK as is, but I worry that folks could easily use the wrong one.
Comment on attachment 54732 [details] Patch Throwing an exception when previously we would silently do nothing could easily be a backward compatibility problem for the Objective-C bindings. > +int HTMLInputElement::selectionStart(ExceptionCode& ec) > +{ > + if (!canHaveSelection()) { > + ec = TYPE_MISMATCH_ERR; > + return -1; > + } I think this gives a different exception than before. Probably the new one is correct and the old was incorrect, but since it's a behavior change I'd like to see tests showing the behavior.
We should be able to have the ObjC bindings call the non-ec version of the method. I'm not sure why that didn't work.
Attachment 54732 [details] was posted by a committer and has review+, assigning to Adam Barth for commit.
Committed r59502: <http://trac.webkit.org/changeset/59502>
I landed the TestObj.idl changes in http://trac.webkit.org/changeset/59502. I'm going to post a new patch that addresses Darin's feedback shortly.
Created attachment 56139 [details] work in progress
Created attachment 56141 [details] Patch
Comment on attachment 56141 [details] Patch Good to avoid custom bindings. Awkward to have these two versions of these basic functions side by side. The only other place I can think of where we have that is the Range class. Note the different way these functions are declared in that class compared to what we have in this patch. I think I like the Range style slightly better. > + // The methods that take ExceptionCodes check canHaveSelection(). Methods is not a C++ term, and I normally avoid using it to mean member function in WebKit code. I would say "The following functions" or "These functions" rather than "The methods".
I'm confused why this is open?