[webkit-reviews] review denied: [Bug 34464] [Chromium] Chromium should receive checkbox state changes : [Attachment 48063] Fixing style - line endings.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 3 22:55:31 PST 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied
chris.guillory at google.com's request for review:
Bug 34464: [Chromium] Chromium should receive checkbox state changes
https://bugs.webkit.org/show_bug.cgi?id=34464

Attachment 48063: Fixing style - line endings.
https://bugs.webkit.org/attachment.cgi?id=48063&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: WebCore/ChangeLog
...
> +
> +	   No new tests.

^^^ please delete the line that says "No new tests."

That is in the template to provide a suggestion to you that you
should consider creating new tests :-)

(For this change, I don't think a new test is needed.)


> Index: WebCore/accessibility/chromium/AXObjectCacheChromium.cpp

> +static ChromeClientChromium* toChromeClientChromium(Widget* widget)

This function should be changed to take a FrameView* as a parameter.
Then you can avoid having to check if the widget is a FrameView :-)


> Index: WebKit/chromium/public/WebViewClient.h
...
> +    // Notifies embedder that the state of an accessibility object has
changed.
> +    virtual void accessibilityObjectStateDidChange(const
WebAccessibilityObject&) { }

I previously suggested didChangeAccessibilityObjectState to match
the naming conventions of other methods found in WebViewClient.h.

Unless you feel strongly about this, I think we should go with
that instead (for consistency).


More information about the webkit-reviews mailing list