Continuing from bug 112993 and bug 112085. Element::disabled is for form controls, and Element::isEnabledFormControl is redundant with Element::disabled, so replace them with a single function Element::isDisabledFormControl.
Created attachment 194998 [details] Patch
Comment on attachment 194998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194998&action=review > Source/WebCore/html/HTMLFormControlElement.h:67 > + virtual bool isDisabledFormControl() const; It's time to add OVERRIDE. > Source/WebCore/html/HTMLOptGroupElement.h:-48 > - virtual bool isEnabledFormControl() const OVERRIDE { return !disabled(); } I feel it worth having isEnabledFormControl() as a inlined shortcut of !isDisabledFormControl(). Doing !isDisabledFormControl() feels like a confusing double-negative.
Comment on attachment 194998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194998&action=review > Source/WebCore/dom/Element.h:561 > + virtual bool isDisabledFormControl() const; nit: If you follow the terminology in the standard, this should be called "isActuallyDisabled." http://www.whatwg.org/specs/web-apps/current-work/multipage/common-idioms.html#concept-element-disabled "actually disabled" is applied to non-form controls too like <menuitem>. You can ignore this comment.
(In reply to comment #2) > > Source/WebCore/html/HTMLOptGroupElement.h:-48 > > - virtual bool isEnabledFormControl() const OVERRIDE { return !disabled(); } > > I feel it worth having isEnabledFormControl() as a inlined shortcut of !isDisabledFormControl(). > Doing !isDisabledFormControl() feels like a confusing double-negative. I think isEnabledFormControl() feels confusing since it returns true for elements that are not form controls.
(In reply to comment #4) > (In reply to comment #2) > > > Source/WebCore/html/HTMLOptGroupElement.h:-48 > > > - virtual bool isEnabledFormControl() const OVERRIDE { return !disabled(); } > > > > I feel it worth having isEnabledFormControl() as a inlined shortcut of !isDisabledFormControl(). > > Doing !isDisabledFormControl() feels like a confusing double-negative. > > I think isEnabledFormControl() feels confusing since it returns true for elements that are not form controls. Makes sense.
Created attachment 195428 [details] add OVERRIDE
Comment on attachment 195428 [details] add OVERRIDE View in context: https://bugs.webkit.org/attachment.cgi?id=195428&action=review > Source/WebCore/html/shadow/TextControlInnerElements.cpp:389 > RefPtr<HTMLInputElement> input(static_cast<HTMLInputElement*>(shadowHost())); > - if (!input || input->isDisabledOrReadOnly()) > return; Looks very wrong.
Created attachment 195452 [details] fix bad merging
Comment on attachment 195452 [details] fix bad merging looks ok
Comment on attachment 195452 [details] fix bad merging Clearing flags on attachment: 195452 Committed r147135: <http://trac.webkit.org/changeset/147135>
All reviewed patches have been landed. Closing bug.