Rename drawSystemFocusRing() to drawFocusIfNeeded()

Rename drawSystemFocusRing() to drawFocusIfNeeded()

RESOLUTION: to change draw SystemFocusRing to drawFocusIfNeeded.

public discussion:
  http://www.w3.org/Bugs/Public/show_bug.cgi?id=23980
  http://www.w3.org/WAI/PF/HTML/wiki/Summary_of_2014-01-13_Meeting
  http://www.w3.org/2014/01/13-html-a11y-minutes.html#item02

other browsers:
  It has already been agreed or changed.
  http://bugzilla.mozilla.org/show_bug.cgi?id=959820 (Firefox)

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

[zino, 2014-03-21 18:46:11]

This change has been suggested by Rik.

I don't know this change is trivial or not.
It seems there is no definition for renaming in the guideline.
However, because it has been hidden behind the experimental runtime flag,
I don't think that I should need "intent to implement".
If I have to need it, please tell me.

Thank you :)

[Justin Novosad, 2014-03-21 19:17:39]

I agree that no intent to implement is needed here.
For reference, the spec bug is here:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23980
Rik: this issue is marked as resolved on the w3 bugzilla, but the change is not
reflected in the WhatWG spec. How do we handle that?

The layout test files should be renamed.

Other than that, this looks fine to me, but I will defer approval to Dominic and
Rik.

[dmazzoni, 2014-03-21 21:39:25]

Agreed, no intent to implement needed. However, we need some more changes as
well:

* Rename the test
* Get rid of the drawCustomFocusRing implementation and tests, that was removed
from the spec as far as I know
* Get rid of the behavior that updates accessibility, that was also removed from
the spec

https://codereview.chromium.org/208003003/diff/1/LayoutTests/fast/canvas/draw...
File LayoutTests/fast/canvas/draw-system-focus-ring-expected.txt (right):

https://codereview.chromium.org/208003003/diff/1/LayoutTests/fast/canvas/draw...
LayoutTests/fast/canvas/draw-system-focus-ring-expected.txt:1: PASS
canvas.drawFocusIfNeeded(null); did not throw exception.
I think you should rename the test too, i.e. call it draw-focus-if-needed

[Rik, 2014-03-21 21:53:57]

On 2014/03/21 19:17:39, junov wrote:
> I agree that no intent to implement is needed here.
> For reference, the spec bug is here:
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=23980
> Rik: this issue is marked as resolved on the w3 bugzilla, but the change is
not
> reflected in the WhatWG spec. How do we handle that?

As soon as you land this patch, we should notify the WhatWG so Ian can update
the spec.

[Rik, 2014-03-21 21:54:22]

On 2014/03/21 21:39:25, dmazzoni wrote:
> Agreed, no intent to implement needed. However, we need some more changes as
> well:
> 
> * Rename the test
> * Get rid of the drawCustomFocusRing implementation and tests, that was
removed
> from the spec as far as I know
> * Get rid of the behavior that updates accessibility, that was also removed
from
> the spec
> 

Correct.

[zino, 2014-03-24 15:41:18]

Thank you for reviews and good informations.
I also think we need some more changes. BTW, It seems that it is better to
seperate CLs.
So, Would you mind if I create another CLs about dCFR and updating accesibility?

And,
I uploaded new patch to change the file name.
Could you please review it again? (only for renaming)

[dmazzoni, 2014-03-24 15:43:20]

lgtm

OK, we can land this and you can follow it with more changes to
remove drawCustomFocusRing and remove the accessibility part.
Thanks.

[commit-bot: I haz the power, 2014-03-24 15:43:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/30002

[commit-bot: I haz the power, 2014-03-24 15:46:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-24 15:46:34]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[zino, 2014-03-25 01:55:02]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-25 01:55:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/30002

[commit-bot: I haz the power, 2014-03-25 01:57:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 01:57:39]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[zino, 2014-03-25 03:00:04]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-25 03:00:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/180001

[commit-bot: I haz the power, 2014-03-25 04:01:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 04:01:50]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[zino, 2014-03-25 04:17:30]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-25 04:17:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/180001

[Rik, 2014-03-25 05:26:45]

On 2014/03/21 21:39:25, dmazzoni wrote:
> * Get rid of the behavior that updates accessibility, that was also removed
from
> the spec

I'm still not convinced that it was out of place. Really too bad since it seemed
like such a natural way...

[commit-bot: I haz the power, 2014-03-25 05:26:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 05:26:53]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[zino, 2014-03-25 05:30:37]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-25 05:30:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/180001

[commit-bot: I haz the power, 2014-03-25 06:38:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 06:38:21]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[zino, 2014-03-25 08:21:54]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-25 08:21:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/208003003/180001

[commit-bot: I haz the power, 2014-03-25 09:20:17]

Change committed as 169925