Add hack so VoiceOver speaks updates to injected live regions.

Add hack so VoiceOver speaks updates to injected live regions.

See bug for repro html, requires manual testing.

BUG=371102
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270104

[David Tseng, 2014-05-08 17:07:01]

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
File content/browser/accessibility/browser_accessibility_mac.mm (right):

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
content/browser/accessibility/browser_accessibility_mac.mm:56:
browser_accessibility_cocoa_ = [[BrowserAccessibilityCocoa alloc]
Could you check for loss of VoiceOver when this code gets hit? (both when
VoiceOver's cursor is over focusable and non-focusable items).

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
File content/browser/accessibility/browser_accessibility_manager_mac.mm (right):

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
content/browser/accessibility/browser_accessibility_manager_mac.mm:159: // the
initial page load work fine.) By recreating the NSAccessibility
Doesn't this code get triggered on initial load as well?

[dmazzoni, 2014-05-08 21:02:05]

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
File content/browser/accessibility/browser_accessibility_mac.mm (right):

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
content/browser/accessibility/browser_accessibility_mac.mm:56:
browser_accessibility_cocoa_ = [[BrowserAccessibilityCocoa alloc]
On 2014/05/08 17:07:02, David Tseng wrote:
> Could you check for loss of VoiceOver when this code gets hit? (both when
> VoiceOver's cursor is over focusable and non-focusable items).

Works great when the cursor is over a focusable item, nothing is spoken and no
context is lost.

When the cursor is over a non-focusable item, it's silent, but when you try to
navigate again it snaps you back to the last focused item first. Not idea, but I
think it's an okay trade-off.

Remember we can lose this hack when the next OS X version comes out.

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
File content/browser/accessibility/browser_accessibility_manager_mac.mm (right):

https://codereview.chromium.org/272573003/diff/1/content/browser/accessibilit...
content/browser/accessibility/browser_accessibility_manager_mac.mm:159: // the
initial page load work fine.) By recreating the NSAccessibility
On 2014/05/08 17:07:02, David Tseng wrote:
> Doesn't this code get triggered on initial load as well?

True. I updated the comment.

[David Tseng, 2014-05-09 03:09:07]

lgtm with nits.

[David Tseng, 2014-05-09 03:09:47]

lgtm with nits.

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_mac.h (right):

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_mac.h:33: void
RecreateNativeObject();
FYI; I forgot that the BAC creates its children (and keeps it as a property).
This method, therefore, would be more appropriately named RecreateNativeSubtree.

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_manager.cc (right):

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager.cc:178:
OnUpdateFinished();
nit: Maybe OnTreeUpdateFinished?

[dmazzoni, 2014-05-09 06:10:29]

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_mac.h (right):

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_mac.h:33: void
RecreateNativeObject();
On 2014/05/09 03:09:47, David Tseng wrote:
> FYI; I forgot that the BAC creates its children (and keeps it as a property).
> This method, therefore, would be more appropriately named
RecreateNativeSubtree.

Oh! Good thought.

Better idea, let's preserve the children? I added some code to swap the
children, could you take another look?

Unfortunately this doesn't make VO stay on static text, but it seems it ought to
be a bit more efficient, right?

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_manager.cc (right):

https://codereview.chromium.org/272573003/diff/20001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager.cc:178:
OnUpdateFinished();
On 2014/05/09 03:09:47, David Tseng wrote:
> nit: Maybe OnTreeUpdateFinished?

Done.

[dmazzoni, 2014-05-12 18:05:35]

Ping - want to take a quick look at the changes I made to preserve children when
recreating the wrapper for the root node?

[David Tseng, 2014-05-12 20:01:51]

https://codereview.chromium.org/272573003/diff/40001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_mac.mm (right):

https://codereview.chromium.org/272573003/diff/40001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_mac.mm:60:
browser_accessibility_cocoa_ = [[BrowserAccessibilityCocoa alloc]
Doesn't it make sense to only recreate the cocoa object if there was an object
previously? (i.e. move this within the if clause).

[David Tseng, 2014-05-12 20:02:04]

Still lgtm

[dmazzoni, 2014-05-13 07:28:19]

https://codereview.chromium.org/272573003/diff/40001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_mac.mm (right):

https://codereview.chromium.org/272573003/diff/40001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_mac.mm:60:
browser_accessibility_cocoa_ = [[BrowserAccessibilityCocoa alloc]
On 2014/05/12 20:01:51, David Tseng wrote:
> Doesn't it make sense to only recreate the cocoa object if there was an object
> previously? (i.e. move this within the if clause).

Done.

[dmazzoni, 2014-05-13 07:28:25]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-13 07:29:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/272573003/60001

[commit-bot: I haz the power, 2014-05-13 10:42:32]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-13 12:03:19]

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

[commit-bot: I haz the power, 2014-05-13 12:03:19]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[dmazzoni, 2014-05-13 14:12:00]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-05-13 14:12:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/272573003/60001

[commit-bot: I haz the power, 2014-05-13 15:23:45]

Change committed as 270104