[webkit-reviews] review granted: [Bug 87380] Form controls in <fieldset disabled> should not be focusable. : [Attachment 144718] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 87380: Form controls in <fieldset disabled> should not be focusable.
https://bugs.webkit.org/show_bug.cgi?id=87380

Attachment 144718: Patch
https://bugs.webkit.org/attachment.cgi?id=144718&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=144718&action=review


> Source/WebCore/html/HTMLFormControlElement.cpp:322
>  bool HTMLFormControlElement::supportsFocus() const
>  {
> -    return !m_disabled;
> +    return !disabled();
>  }

This code change shows that our naming in this class is not good! It's terrible
to be in a situation where m_disabled is one thing and disabled() another, and
switching from one to the other fixes a bug. The names should reflect
differences between the two.

Our longer term direction should be to give good, clear names to this class's
members. If the names from the DOM standard are causing us problems then we
should think about solutions to that; maybe we could start putting prefixes on
some names used in DOM bindings to make sure no one is tempted to use them
inside WebKit itself.

Does this code change have any other effects? Is being wrapped inside a
disabled fieldset the only difference between disabled() and m_disabled. Are
there tests showing these other effects are OK?