[webkit-reviews] review denied: [Bug 132584] Add support for drawFocusIfNeeded : [Attachment 231037] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 8 00:35:34 PDT 2014


Dirk Schulze <krit at webkit.org> has denied Rik Cabanier <cabanier at adobe.com>'s
request for review:
Bug 132584: Add support for drawFocusIfNeeded
https://bugs.webkit.org/show_bug.cgi?id=132584

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

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=231037&action=review


r- because I asked for the changes last review. Patch itself looks good though.


> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1985
> +    if (!element)
> +	   return;
> +    if (!state().m_hasInvertibleTransform)
> +	   return;
> +    if (m_path.isEmpty())
> +	   return;
> +    if (!element->isDescendantOf(canvas()))
> +	   return;
> +    
> +    if (element->focused()) {

you can collapse those into one if clause. And (as said in previous review!)
make it if (!element->focused()) return

> LayoutTests/fast/canvas/draw-focus-if-needed.html:3
> +    <button id="button1"></button>

Change all tabs to spaces.

> LayoutTests/fast/canvas/draw-focus-if-needed.html:23
> +else
> +	{

else {

> LayoutTests/fast/canvas/draw-focus-if-needed.html:34
> +	}

no indentation


More information about the webkit-reviews mailing list