Issue metadata
Sign in to add a comment
|
MacViews: Voiceover focus follows mouse doesn't work |
||||||||||||||||||||||||
Issue descriptionWhen navigating from the omnibox, VoiceOver reads "Chrome has new window", and keyboard focus lands on whatever this new window is. This breaks VoiceOver focus-follows-mouse and is probably quite confusing. Background: Focus follows mouse is a VoiceOver setting, enabled via VoiceOver Utility > Navigation > Mouse "Moves VoiceOver Cursor" - This has never worked in MacViews top chrome - It formerly "worked" in MacViews web content, but was very fiddly - Now it doesn't seem to work at all - I bisected the current status to the unified compositing change that caused issue 852781 - ...but the fix for that issue doesn't appear to fix it - Part of the problem may be that RenderWidgetHostViewCocoa is currently semi-detached from the MacViews accessibility tree (see discussion in Issue 833638). Launching Chromium from the console while attempting to use it with focus-follow-mouse reveals a bunch of error messages from AXPlatformNodeCocoa attempting to hit test into RenderWidgetHostViewCocoa. - ...but fixing this doesn't cause focus-follows-mouse to work - When tabbing through top chrome, VoiceOver focus becomes detached from system focus when tabbing into web content (see Issue 650118 )
,
Jul 10
I tried bisecting this and the gist is: focus follows mouse has been some form of broken or another the whole time and it's not clear to me how to quantify the brokenness for any particular build since it seems to be semi-random re: whether - Focusing/unfocusing web content - Typing in the omnibox and cancelling - Navigating via omnibox or link - Turning VO on or off helps or hurts. One thing I can say is that it never worked in top chrome besides the non-refresh profile switcher button (which happens to live in the non-client frame view, so probably not a coincidence). Testing this in developer builds throws a bunch of exceptions like the one in Issue 833638 (but non-fatally), which makes me think that at minimum, we need to fix the situation where the accessibility tree is not coherent between the top chrome and web content. But, I'm not quite sure if this is enough.
,
Jul 12
,
Jul 12
,
Jul 12
+dmazzoni@ in case you have some insight as you did in https://codereview.chromium.org/2210763002 I have a local build that correctly wires RenderWidgetHostViewCocoa and its BrowserAccessibilityCocoa into the views accessibility tree but it doesn't seem to make a difference for this. As far as I can tell, VO knows what should be focused, hit testing is correct etc, but still nada.
,
Jul 12
,
Jul 13
Changing title to reflect my current understanding: Background: Focus follows mouse is a VoiceOver setting, enabled via VoiceOver Utility > Navigation > Mouse "Moves VoiceOver Cursor" - This has never worked in MacViews top chrome - It formerly "worked" in MacViews web content, but was very fiddly - Now it doesn't seem to work at all - I bisected the current status to the unified compositing change that caused issue 852781 - ...but the fix for that issue doesn't appear to fix it - Part of the problem may be that RenderWidgetHostViewCocoa is currently semi-detached from the MacViews accessibility tree (see discussion in Issue 833638). Launching Chromium from the console while attempting to use it with focus-follow-mouse reveals a bunch of error messages from AXPlatformNodeCocoa attempting to hit test into RenderWidgetHostViewCocoa. - ...but fixing this doesn't cause focus-follows-mouse to work - When tabbing through top chrome, VoiceOver focus becomes detached from system focus when tabbing into web content (see Issue 650118 )
,
Jul 13
,
Jul 16
I'm not experiencing the issue with it being fiddly in web content with MacViews disabled. Can you clarify what doesn't work?
,
Jul 16
FWIW, this is actually the main change where I added support for this feature: https://codereview.chromium.org/2393123002 The only thing I "fixed" was the implementation of accessibilityHitTest in BrowserAccessibilityCocoa, which is called from RWHVCocoa::accessibilityHitTest (pre-MacViews)
,
Jul 17
Load balancing.
,
Jul 17
,
Jul 17
,
Jul 18
The Accessibility Inspector is also unable to follow the mouse focused elements in Chromium.
,
Jul 18
Current theory: An accessible element seems to be covering the entire region. Removing this element might just make things work.
,
Jul 18
Readding lgrey@
,
Jul 18
I think I know why you would say this (the VO cursor starts covering the whole window), but in my testing, the hit tests returned correctly (which...doesn't mean you're wrong! VO is confusing).
Here's my hacky LLDB command for dumping the tree. (Usage: get the main window's pointer
and call like "ax_tree 0x8badf00d"). It can be easily adapted to find elements that have the same size as the window.
def result_is_success(result):
error = result.GetError()
return error == 0x1001 or error.Success()
def responds_to_selector(frame, pointer, selector):
expression = "[(NSObject*)%s respondsToSelector:@selector(%s)]" % (pointer, selector)
result = frame.EvaluateExpression(expression)
return result.GetError().Success() and result.GetValueAsUnsigned() == 1
def description(frame, pointer):
return frame.EvaluateExpression(pointer).GetObjectDescription()
def accessibility_children(frame, pointer):
if not responds_to_selector(frame, pointer, "accessibilityAttributeValue:"):
return []
expression = "(NSArray*)[(NSObject*)%s performSelector:@selector(accessibilityAttributeValue:) withObject:@\"AXChildren\"]" % pointer
result = frame.EvaluateExpression(expression)
if not result_is_success(result):
return []
return (val.GetValue() for val in result if val.IsValid() and result_is_success(val))
def print_ax_tree(frame, pointer, level=0):
print "\t" * level + description(frame, pointer)
for child in accessibility_children(frame, pointer):
print_ax_tree(frame, child, level + 1)
# Change `lldb_commands` to your module here
debugger.HandleCommand('command script add -f lldb_commands.ax_tree ax_tree')
One other thing that may come up: Our root view's AX node currently has the window role, which makes the hierarchy
Window (NSWindow)
Window (RootView)
...
I tried changing the root view to group, but it didn't help.
,
Jul 18
Whoops, the above also needs
def get_selected_frame(debugger):
return debugger.GetSelectedTarget().GetProcess().GetSelectedThread().GetSelectedFrame()
def ax_tree(debugger, command, result, internal_dict):
print_ax_tree(get_selected_frame(debugger), command)
,
Jul 18
I've been using the Accessibility Inspector to get a look at what macOS thinks is there, and it seems to suggest two overlapping elements.
,
Jul 18
Parent and Child2 are ignored though!
,
Jul 18
Ignored doesn't seem to mean non-traversable. In Safari, there are a lot of nodes that are "ignored" but are permitted to have accessible elements as long as they hit test correctly.
,
Jul 18
Sure, we do this also. I think it's pretty unlikely that ignored nodes are relevant, though, especially since we ignore them in hittesting via NSAccessibilityUnignoredAncestor/NSAccessibilityUnignoredChildren
,
Jul 18
The latest patchset to https://chromium-review.googlesource.com/c/chromium/src/+/1138793 semi-accidentally fixes this.
,
Jul 18
Agreed. With https://chromium-review.googlesource.com/c/chromium/src/+/1138793/1..7 It works a lot better. Sending this back over to lgrey to land.
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5779245ee8c0476167fea8733b23102d5b280c4d commit 5779245ee8c0476167fea8733b23102d5b280c4d Author: Leonard Grey <lgrey@chromium.org> Date: Thu Jul 19 14:24:43 2018 MacViews: Explicitly stitch together content and views a11y hierarchies Also fixes assumption in AXPlatformNodeCocoa that all children are also AXPlatformNodeCocoa Bug: 861756 , 833638 Change-Id: I3fbf05858b00f8a8ee06d8c5d0af988252ca26a8 Reviewed-on: https://chromium-review.googlesource.com/1138793 Commit-Queue: Leonard Grey <lgrey@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#576479} [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/content/browser/web_contents/web_contents_view_mac.h [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/content/browser/web_contents/web_contents_view_mac.mm [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/content/browser/web_contents/web_contents_view_mac_unittest.mm [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/ui/accessibility/platform/ax_platform_node_mac.mm [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/ui/base/BUILD.gn [add] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/ui/base/cocoa/accessibility_hostable.h [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/ui/views/controls/native/native_view_host_mac.mm [modify] https://crrev.com/5779245ee8c0476167fea8733b23102d5b280c4d/ui/views/controls/native/native_view_host_mac_unittest.mm
,
Jul 19
,
Jul 19
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
,
Jul 20
,
Jul 23
Able to reproduce the issue on chrome reported version 69.0.3487.0(Build without fix) Verified the fix on Mac 10.13.5 on Chrome version #70.0.3500.0 as per the comment#0 Attaching screenshot for reference. Observed "able to see focus-follows-mouse to work" Hence, the fix is working as expected. Adding the verified label. Thanks! |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by ccameron@chromium.org
, Jul 9