Make the matching of root dialogs more strict

Make the matching of root dialogs more strict

In the views tree, dialogs as annotated today are fairly loose semantically.

1. In the Launcher, we're using dialog for bubble views.
Example:
window
  ...
  search text field
...
window
  ...
  dialog
    ...
    dialog
      files app button

The problem arises when trying to wrap. It is not clear if we should wrap based on the inner dialog, the outer dialog, or the outer window.

Furthermore, consider the views tree for app installation

window
  dialog
  ...
    close button
  ...
  dialog
    description text static text
  ...
  install button

Here, we should not wrap for the inner dialog because we'll miss the buttons that come after.

Thus, we need to be far more strict when detecting dialogs as root. In particular, dialogs that are not parented to windows and have siblings that are not all windows or dialogs, should not be a root.

TEST=navigate both app install window and launcher. Verify that wrapping only occurs for the launcher case and not for the install window.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2573203002
Cr-Commit-Position: refs/heads/master@{#443098}
Committed: https://chromium.googlesource.com/chromium/src/+/b190dd7e0396cdc47e4444bab5618500c470c80e

[David Tseng, 2016-12-14 20:08:45]

Description was changed from

==========
Make the matching of root dialogs more strict

In the views tree, dialogs as annotated today are fairly loose semantically.

1. In the Launcher, we're using dialog for bubble views.
Example:
window
  ...
  search text field
...
window
  ...
  dialog
    ...
    dialog
      files app button

The problem arises when trying to wrap. It is not clear if we should wrap based
on the inner dialog, the outer dialog, or the outer window.

Furthermore, consider the views tree for app installation

window
  dialog
  ...
    close button
  ...
  dialog
    description text static text
  ...
  install button

Here, we should not wrap for the inner dialog because we'll miss the buttons
that come after.

Thus, we need to be far more strict when detecting dialogs as root. In
particular, dialogs that are not parented to windows and have siblings that are
not all windows or dialogs, should not be a root.

TEST=navigate both app install window and launcher. Verify that wrapping only
occurs for the launcher case and not for the install window.
BUG=
==========

to

==========
Make the matching of root dialogs more strict

In the views tree, dialogs as annotated today are fairly loose semantically.

1. In the Launcher, we're using dialog for bubble views.
Example:
window
  ...
  search text field
...
window
  ...
  dialog
    ...
    dialog
      files app button

The problem arises when trying to wrap. It is not clear if we should wrap based
on the inner dialog, the outer dialog, or the outer window.

Furthermore, consider the views tree for app installation

window
  dialog
  ...
    close button
  ...
  dialog
    description text static text
  ...
  install button

Here, we should not wrap for the inner dialog because we'll miss the buttons
that come after.

Thus, we need to be far more strict when detecting dialogs as root. In
particular, dialogs that are not parented to windows and have siblings that are
not all windows or dialogs, should not be a root.

TEST=navigate both app install window and launcher. Verify that wrapping only
occurs for the launcher case and not for the install window.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2016-12-14 20:11:14]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[dmazzoni, 2016-12-15 16:57:45]

lgtm

Change description makes sense, would like to see a short
comment in the code so we remember why that logic was
needed.

https://codereview.chromium.org/2573203002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_predicate.js
(right):

https://codereview.chromium.org/2573203002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_predicate.js:328:
return node.root.role != Role.desktop ||
Please add a concise comment here explaining the purpose of this logic

[dmazzoni, 2016-12-15 16:57:47]

lgtm

Change description makes sense, would like to see a short
comment in the code so we remember why that logic was
needed.

[David Tseng, 2016-12-16 21:58:34]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2016-12-16 21:58:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2573203002/#ps20001
(title: "Make the matching of root dialogs more strict")

[commit-bot: I haz the power, 2016-12-16 22:00:04]

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

[commit-bot: I haz the power, 2016-12-17 00:04:11]

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

[commit-bot: I haz the power, 2016-12-17 00:04:12]

Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[David Tseng, 2017-01-11 22:54:05]

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

[commit-bot: I haz the power, 2017-01-11 22:55:02]

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

[commit-bot: I haz the power, 2017-01-12 00:21:26]

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

[commit-bot: I haz the power, 2017-01-12 00:21:27]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2017-01-12 00:39:41]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2017-01-12 00:40:39]

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

[commit-bot: I haz the power, 2017-01-12 01:05:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484181581665600,
"parent_rev": "50dfde3153a055f19e836c1400d697e1e95ce954", "commit_rev":
"b190dd7e0396cdc47e4444bab5618500c470c80e"}

[commit-bot: I haz the power, 2017-01-12 01:06:00]

Description was changed from

==========
Make the matching of root dialogs more strict

In the views tree, dialogs as annotated today are fairly loose semantically.

1. In the Launcher, we're using dialog for bubble views.
Example:
window
  ...
  search text field
...
window
  ...
  dialog
    ...
    dialog
      files app button

The problem arises when trying to wrap. It is not clear if we should wrap based
on the inner dialog, the outer dialog, or the outer window.

Furthermore, consider the views tree for app installation

window
  dialog
  ...
    close button
  ...
  dialog
    description text static text
  ...
  install button

Here, we should not wrap for the inner dialog because we'll miss the buttons
that come after.

Thus, we need to be far more strict when detecting dialogs as root. In
particular, dialogs that are not parented to windows and have siblings that are
not all windows or dialogs, should not be a root.

TEST=navigate both app install window and launcher. Verify that wrapping only
occurs for the launcher case and not for the install window.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Make the matching of root dialogs more strict

In the views tree, dialogs as annotated today are fairly loose semantically.

1. In the Launcher, we're using dialog for bubble views.
Example:
window
  ...
  search text field
...
window
  ...
  dialog
    ...
    dialog
      files app button

The problem arises when trying to wrap. It is not clear if we should wrap based
on the inner dialog, the outer dialog, or the outer window.

Furthermore, consider the views tree for app installation

window
  dialog
  ...
    close button
  ...
  dialog
    description text static text
  ...
  install button

Here, we should not wrap for the inner dialog because we'll miss the buttons
that come after.

Thus, we need to be far more strict when detecting dialogs as root. In
particular, dialogs that are not parented to windows and have siblings that are
not all windows or dialogs, should not be a root.

TEST=navigate both app install window and launcher. Verify that wrapping only
occurs for the launcher case and not for the install window.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2573203002
Cr-Commit-Position: refs/heads/master@{#443098}
Committed:
https://chromium.googlesource.com/chromium/src/+/b190dd7e0396cdc47e4444bab561...
==========

[commit-bot: I haz the power, 2017-01-12 01:06:01]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b190dd7e0396cdc47e4444bab561...