<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is called

<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is called

Note: things like <button tabindex="-1"> still jack focus unexpectedly (IMO).

R=falken@chromium.org,tkent@chromium.org
BUG=656924

[Dan Beam, 2016-10-19 04:53:04]

Patchset #2 (id:20001) has been deleted

[Dan Beam, 2016-10-19 04:53:08]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2016-10-19 04:56:26]

Description was changed from

==========
<dialog>: don't focus tabindex < 0 elements when .showModal() is called

R=falken@chromium.org
BUG=656924
==========

to

==========
<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is
called

R=falken@chromium.org
BUG=656924
==========

[Dan Beam, 2016-10-19 05:01:08]

Description was changed from

==========
<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is
called

R=falken@chromium.org
BUG=656924
==========

to

==========
<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is
called

Note: things like <button tabindex="-1"> still jack focus unexpectedly (IMO).

R=falken@chromium.org
BUG=656924
==========

[Dan Beam, 2016-10-19 05:01:23]

Description was changed from

==========
<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is
called

Note: things like <button tabindex="-1"> still jack focus unexpectedly (IMO).

R=falken@chromium.org
BUG=656924
==========

to

==========
<dialog>: don't focus non-form elements with tabindex < 0 when showModal() is
called

Note: things like <button tabindex="-1"> still jack focus unexpectedly (IMO).

R=falken@chromium.org,tkent@chromium.org
BUG=656924
==========

[Dan Beam, 2016-10-19 05:01:24]

dbeam@chromium.org changed reviewers:
+ tkent@chromium.org

[Dan Beam, 2016-10-19 05:01:34]

falken@ cuz dialog
tkent@ cuz owners

[Dan Beam, 2016-10-19 05:02:00]

/cc esprehn@ and dpapad@ as fyi

[Dan Beam, 2016-10-19 05:02:42]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 05:06:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-10-19 05:23:29]

<button tabindex=-1> is excluded from sequential focus navigation, but is still
a focusable area.

https://html.spec.whatwg.org/multipage/forms.html#dialog-focusing-steps
> If there isn't one, then let control be the first non-inert focusable area in
subject's control group.

https://html.spec.whatwg.org/multipage/interaction.html#control-group
> Focusable areas in control groups are ordered relative to the tree order of
their DOM anchors. Focusable areas with the same DOM anchor in a control group
are ordered relative to their CSS box's relative positions in a pre-order,
depth-first traversal of the box tree. [CSS]

So, the current behavior looks correct according to the standard.

[falken, 2016-10-19 06:15:30]

On 2016/10/19 05:23:29, tkent wrote:
> <button tabindex=-1> is excluded from sequential focus navigation, but is
still
> a focusable area.
> 
> https://html.spec.whatwg.org/multipage/forms.html#dialog-focusing-steps
> > If there isn't one, then let control be the first non-inert focusable area
in
> subject's control group.
> 
> https://html.spec.whatwg.org/multipage/interaction.html#control-group
> > Focusable areas in control groups are ordered relative to the tree order of
> their DOM anchors. Focusable areas with the same DOM anchor in a control group
> are ordered relative to their CSS box's relative positions in a pre-order,
> depth-first traversal of the box tree. [CSS]
> 
> So, the current behavior looks correct according to the standard.

I spoke with Dan Beam offline and although the current spec mandates focusing
these elements when opening a dialog it sounds like it's not what developers
would expect.

Dan: Could you link to https://github.com/whatwg/html/issues/1929 in the CL
description?

https://codereview.chromium.org/2428333002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/dialog-focus.html
(right):

https://codereview.chromium.org/2428333002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement/dialog-focus.html:23:
<!-- TODO(dbeam): add form controls and anchors with negative tabindex.
Can the TODO include information about when it is actionable? And why not add
these now?

[commit-bot: I haz the power, 2016-10-19 11:09:19]

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

[commit-bot: I haz the power, 2016-10-19 11:09:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)