New issue
Advanced search Search tips

Issue 861756 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

MacViews: Voiceover focus follows mouse doesn't work

Project Member Reported by ellyjo...@chromium.org, Jul 9

Issue description

When 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 )
 
There was some VoiceOver breakage earlier due to  issue 852781  (IIUC things were working after that was fixed, but I'm not a VoiceOver power-user).
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.
Labels: -M-69
Labels: M-69
Cc: dmazz...@chromium.org
+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.
Labels: ReleaseBlock-Stable
Summary: MacViews: Voiceover mouse follows focus doesn't work (was: VoiceOver vs windows created when navigating)
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 )
Summary: MacViews: Voiceover focus follows mouse doesn't work (was: MacViews: Voiceover mouse follows focus doesn't work)
I'm not experiencing the issue with it being fiddly in web content with MacViews disabled. Can you clarify what doesn't work?

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)


Owner: robliao@chromium.org
Load balancing.
Labels: Group-Accessibility
Description: Show this description
The Accessibility Inspector is also unable to follow the mouse focused elements in Chromium.
Current theory: An accessible element seems to be covering the entire region. Removing this element might just make things work.
Cc: lgrey@chromium.org
Readding lgrey@
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.
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)

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.
Parent.png
95.0 KB View Download
Child 1.png
94.8 KB View Download
Child 2.png
124 KB View Download
Parent and Child2 are ignored though!
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. 
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
The latest patchset to https://chromium-review.googlesource.com/c/chromium/src/+/1138793 semi-accidentally fixes this.
Cc: -lgrey@chromium.org robliao@chromium.org
Owner: lgrey@chromium.org
Status: Started (was: Assigned)
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.
Project Member

Comment 25 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Labels: Merge-TBD
[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.
Labels: -Merge-TBD
M69 branch #3497, branched at chromium revision #576753. No merge needed. 
Labels: TE-Verified-M70 TE-Verified-70.0.3500.0
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!
861756.mp4
866 KB View Download

Sign in to add a comment