drawSystemFocusRing should take canvas transformations into account.

drawSystemFocusRing should take canvas transformations into account.

In addition, the accessible rect needs to be applied to ancesetor elements
within the canvas, otherwise their bounding boxes may be clipped.

BUG=261998
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156371

[Stephen White, 2013-08-19 18:08:04]

LGTM; my questions may be dumb

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2396:
elementRect.moveBy(canvasRect.location());
Not new to this patch, but I wonder if this handles weird cases like CSS 3D
transform applied to canvas?

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2402: while (obj &&
!obj->isCanvas()) {
Shouldn't we actually compare against this canvas element? What would happen if
we called this API for an element under a different canvas anyway?

[dmazzoni, 2013-08-20 06:01:32]

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2396:
elementRect.moveBy(canvasRect.location());
On 2013/08/19 18:08:04, Stephen White wrote:
> Not new to this patch, but I wonder if this handles weird cases like CSS 3D
> transform applied to canvas?

I wasn't sure, so I checked with a simple transform and a 3-D transform and it
appeared to work correctly both times!

https://codereview.chromium.org/23314002/diff/1/Source/core/html/canvas/Canva...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2402: while (obj &&
!obj->isCanvas()) {
On 2013/08/19 18:08:04, Stephen White wrote:
> Shouldn't we actually compare against this canvas element? What would happen
if
> we called this API for an element under a different canvas anyway?

Agreed, I wasn't thinking about those corner cases. Fixed.

If you called it with the wrong canvas, it should fail in focusRingCallIsValid
because the element isn't a descendant of the canvas.

However, it's possible you could do something silly like put a canvas in a
canvas (yo dawg), in which case it shouldn't crash, it just probably won't work.

[commit-bot: I haz the power, 2013-08-20 06:01:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/23314002/6001

[commit-bot: I haz the power, 2013-08-20 06:16:13]

Retried try job too often on linux_blink_rel for step(s) webkit_lint,
webkit_python_tests, webkit_tests, webkit_unit_tests, weborigin_unittests,
wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-08-20 06:36:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/23314002/6001

[commit-bot: I haz the power, 2013-08-20 07:34:55]

Change committed as 156371