Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(306)

Issue 2514583002: Ensure fake alert window attached to AXAura tree and refine alert dialog output (Closed)

Created:
4 years, 1 month ago by David Tseng
Modified:
4 years, 1 month ago
Reviewers:
dmazzoni
CC:
chromium-reviews, alemate+watch_chromium.org, sadrul, oshima+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, arv+watch_chromium.org, dtseng+watch_chromium.org, kalyank, dmazzoni+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Ensure fake alert window attached to AXAura tree and refine alert dialog output - in the AXAura tree, we use a fake (undrawn) window to support raw text string alerts from callers. Previously, we were not attaching the window properly as a child of desktop. This resulted in some inconsistencies in the client (extension side) tree. Fix it by always adding this window as a child of desktop - output for the permissions bubble is strange at the moment because the entire subtree is strange: - a window contains descendants dialog, alert dialog. The dialog itself contains duplicate text with the dialog's name. The alert dialog contains a name identical to the dialog, but contains the actual content and the allow, deny buttons. This cl doesn't change the structure, but does allow the user to descend properly into the alert dialog with linear navigation. We do this by making an alert dialog a structural container (i.e. something we descend into when navigating next). The bubble looks like: node id=364 role=window state={} parentID=6 childIds=[359] node id=359 role=dialog state={} parentID=364 childIds=[363] node id=363 role=dialog state={"focused":true} parentID=359 childIds=[362] node id=362 role=client state={} parentID=363 childIds=[367,361] node id=367 role=client state={} parentID=362 childIds=[368,369,358] node id=368 role=image state={} parentID=367 childIds=[] node id=369 role=staticText state={"readOnly":true} parentID=367 childIds=[] node id=358 role=button state={} parentID=367 childIds=[370,371] node id=370 role=image state={} parentID=358 childIds=[] node id=371 role=staticText state={"readOnly":true} parentID=358 childIds=[] node id=361 role=client state={} parentID=362 childIds=[360,365,366] node id=360 role=alertDialog state={} parentID=361 childIds=[372] node id=372 role=unknown state={} parentID=360 childIds=[373,374] node id=373 role=unknown state={} parentID=372 childIds=[375,376] node id=375 role=image state={} parentID=373 childIds=[] node id=376 role=staticText state={"readOnly":true} parentID=373 childIds=[] node id=374 role=unknown state={} parentID=372 childIds=[] node id=365 role=button state={"focusable":true} parentID=361 childIds=[377,378] node id=377 role=image state={} parentID=365 childIds=[] node id=378 role=staticText state={"readOnly":true} parentID=365 childIds=[] node id=366 role=button state={"focusable":true} parentID=361 childIds=[379,380] node id=379 role=image state={} parentID=366 childIds=[] node id=380 role=staticText state={"readOnly":true} parentID=366 childIds=[] TEST=visit site containing notification; ensure we get focused on that window and search+right visits all window elements. Use ChromeVox for a while and cause an alert (e.g. press caps lock). Ensure we don't get any errors concerning invalid nodes on deserialization and that ChromeVox only alerts once. BUG=619266 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/319f110637783ce562ad6092120441756572e644 Cr-Commit-Position: refs/heads/master@{#433321}

Patch Set 1 #

Patch Set 2 : Fix test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -6 lines) Patch
M chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_predicate.js View 2 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (13 generated)
David Tseng
4 years, 1 month ago (2016-11-17 22:09:53 UTC) #3
dmazzoni
lgtm
4 years, 1 month ago (2016-11-17 23:10:48 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2514583002/1
4 years, 1 month ago (2016-11-18 15:27:33 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/275667)
4 years, 1 month ago (2016-11-18 17:52:06 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2514583002/20001
4 years, 1 month ago (2016-11-18 18:51:58 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-18 22:44:53 UTC) #18
commit-bot: I haz the power
4 years, 1 month ago (2016-11-18 22:55:26 UTC) #20
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/319f110637783ce562ad6092120441756572e644
Cr-Commit-Position: refs/heads/master@{#433321}

Powered by Google App Engine
This is Rietveld 408576698