[Webkit-unassigned] [Bug 36562] [Chromium] Implement WebFormControlElement and WebSelectElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36562


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51564|review?                     |review-
               Flag|                            |




--- Comment #9 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-03-25 15:20:07 PST ---
(From update of attachment 51564)
> Index: WebKit/chromium/public/WebElement.h
...
> +        WEBKIT_API bool isEnabledFormControl() const;

I wonder... could this just be isFormControl()?  Then, on
WebFormControlElement, could we have an isEnabled() method?

I realize that you are just replicating what WebCore does.
Would it be hard to support the interface I'm recommending?
It seems cleaner to me.


> Index: WebKit/chromium/public/WebFormControlElement.h

> +    WebFormControlElement(const WebFormControlElement& n) : WebElement(n) { }
> +
> +    WebFormControlElement& operator=(const WebFormControlElement& n) { WebElement::assign(n); return *this; }
> +    WEBKIT_API void assign(const WebFormControlElement& n) { WebElement::assign(n); }

nit: n -> e ("e for element")


> +    WEBKIT_API WebString formControlName() const;
> +    WEBKIT_API WebString formControlType() const;
> +    // Returns the name that should be used for the specified |element| when

nit: insert a new line above this comment


> Index: WebKit/chromium/public/WebInputElement.h
...
> +        WebInputElement(const WebInputElement& n) : WebFormControlElement(n) { }
>  
> +        WebInputElement& operator=(const WebInputElement& n) { WebFormControlElement::assign(n); return *this; }
> +        WEBKIT_API void assign(const WebInputElement& n) { WebFormControlElement::assign(n); }

nit: n -> e ("e for element")


> Index: WebKit/chromium/public/WebSelectElement.h

> +    WebSelectElement(const WebSelectElement& n) : WebFormControlElement(n) { }
> +
> +    WebSelectElement& operator=(const WebSelectElement& n) { WebFormControlElement::assign(n); return *this; }
> +    WEBKIT_API void assign(const WebSelectElement& n) { WebFormControlElement::assign(n); }

nit: n -> e ("e for element")


> +
> +    WEBKIT_API void setValue(const WebString& value);

nit: webkit style says to not name the parameter here since the name
doesn't add information.


> Index: WebKit/chromium/src/WebSelectElement.cpp

> +void WebSelectElement::setAutofilled(bool)
> +{
> +
> +}

^^^ should this have an implementation?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.