Uses the activedescendant_changed event received from Blink to fire the right focus event

Uses the activedescendant_changed event received from Blink to fire the right focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role) to fire activedescendant_changed events when the current selection changes. This behavior is the same as with ARIA combo boxes and other ARIA-based widgets that can have an active descendant. This means that we now have a uniform way of notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active descendant when it changes, instead of relying on Blink.
3. Fires a focus event on the whole list or combo box when it collapses to make Windows screen readers enable their virtual cursor again.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests

Committed: https://crrev.com/2c4ec4bed7e28e4f45efe742833dd3a154ac266b
Cr-Commit-Position: refs/heads/master@{#396653}

[dmazzoni, 2016-04-18 18:27:04]

I'd love to see some tests for this in content/test/data/accessibility/event/

https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager.cc (right):

https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager.cc:455: if (!focus
|| !focus->instance_active())
We shouldn't ever have to check instance_active() here.

The only time we need to check instance_active is in the implementation of a
native API function, like get_accChild, etc.

When instance_active() is false, we've already removed all references to the
object, it's completely unreachable - the only remaining references are held by
another process.

https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager_mac.mm (left):

https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager_mac.mm:153: if
(node->GetRole() == ui::AX_ROLE_LIST_BOX_OPTION &&
Deleting this code will break list boxes on Mac. VoiceOver is silent if you fire
focus events on list box options, try it.

[chromium-reviews, 2016-04-19 17:39:37]

I am testing on the Mac, but you can review my additional event tests in 
the meantime.

On 4/18/2016 2:27 PM, dmazzoni@chromium.org wrote:
> I'd love to see some tests for this in 
> content/test/data/accessibility/event/
>
>
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_manager.cc
> (right):
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_manager.cc:455: if
> (!focus || !focus->instance_active())
> We shouldn't ever have to check instance_active() here.
>
> The only time we need to check instance_active is in the implementation
> of a native API function, like get_accChild, etc.
>
> When instance_active() is false, we've already removed all references to
> the object, it's completely unreachable - the only remaining references
> are held by another process.
>
Done.

>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_manager_mac.mm
> (left):
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_manager_mac.mm:153:
> if (node->GetRole() == ui::AX_ROLE_LIST_BOX_OPTION &&
> Deleting this code will break list boxes on Mac. VoiceOver is silent if
> you fire focus events on list box options, try it.

But with the changes I made to Blink, list box options should not 
receive focus events any more.
I am testing on the Mac and I'll make any required changes.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-04-19 18:23:52]

I think you forgot to upload again?

On Tue, Apr 19, 2016 at 10:39 AM Nektarios Paisios <nektar@google.com>
wrote:

> I am testing on the Mac, but you can review my additional event tests in
> the meantime.
>
>
> On 4/18/2016 2:27 PM, dmazzoni@chromium.org wrote:
>
> I'd love to see some tests for this in
> content/test/data/accessibility/event/
>
>
>
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_manager.cc
> (right):
>
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_manager.cc:455: if
> (!focus || !focus->instance_active())
> We shouldn't ever have to check instance_active() here.
>
> The only time we need to check instance_active is in the implementation
> of a native API function, like get_accChild, etc.
>
> When instance_active() is false, we've already removed all references to
> the object, it's completely unreachable - the only remaining references
> are held by another process.
>
> Done.
>
>
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_manager_mac.mm
> (left):
>
>
>
https://codereview.chromium.org/1899823002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_manager_mac.mm:153:
> if (node->GetRole() == ui::AX_ROLE_LIST_BOX_OPTION &&
> Deleting this code will break list boxes on Mac. VoiceOver is silent if
> you fire focus events on list box options, try it.
>
>
> But with the changes I made to Blink, list box options should not receive
> focus events any more.
> I am testing on the Mac and I'll make any required changes.
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-04-20 18:56:18]

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

[commit-bot: I haz the power, 2016-04-20 18:56:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/20001

[nektarios, 2016-04-20 19:04:00]

Description was changed from

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

BUG=593646
R=dmazzoni@chromium.org
==========

to

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
3. Modified browser code to fire a focus event on the widget itself when a
selection has been made and the widget becomes collapsed.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests, layout tests
==========

[chromium-reviews, 2016-04-20 19:05:34]

On 4/19/2016 2:23 PM, Dominic Mazzoni wrote:
> I think you forgot to upload again?
>

There was a problem with "git cl upload". I finally managed to solve it 
after getting help from chromium-dev.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-04-20 20:09:00]

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

[commit-bot: I haz the power, 2016-04-20 20:09:01]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dmazzoni, 2016-04-21 16:20:08]

https://codereview.chromium.org/1899823002/diff/20001/content/test/data/acces...
File
content/test/data/accessibility/event/aria-combo-box-collapse-expected-mac.txt
(right):

https://codereview.chromium.org/1899823002/diff/20001/content/test/data/acces...
content/test/data/accessibility/event/aria-combo-box-collapse-expected-mac.txt:2:
AXFocusedUIElementChanged on AXList
Do you know why we're getting two focus events? I thought the recent logic I
added to browser_accessibility_manager should have prevented two focus events on
the same node in a row

[chromium-reviews, 2016-04-26 16:40:39]

I fixed the multiple focus events.
|FireFocusEventsWhenNeeded| was being called before the loop that 
processes all events, hence we were getting the focus event twice.
I think we should check this in, and the remaining two issues I will do 
in another patch.
1. Focus event should fire after a combo box is collapsed using alt+up 
so that Jaws / NVDA would switch to Browse Mode.
2. Figure out how to make cursor left / right work when search 
suggestions are up, i.e. when a combo box is open.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-04-26 16:41:07]

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

[commit-bot: I haz the power, 2016-04-26 16:41:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/40001

[nektarios, 2016-04-26 16:42:44]

Description was changed from

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
3. Modified browser code to fire a focus event on the widget itself when a
selection has been made and the widget becomes collapsed.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests, layout tests
==========

to

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests
==========

[commit-bot: I haz the power, 2016-04-26 17:53:15]

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

[commit-bot: I haz the power, 2016-04-26 17:53:16]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dmazzoni, 2016-04-26 20:24:25]

the blink changes look good, but more comments about the Chrome changes

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_manager_mac.mm (right):

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_mac.mm:167:
native_node = ToBrowserAccessibilityCocoa(focus);
I wonder, is this maybe not necessary now?

After an active descendant changed event, we'll compute the new focused node and
call FireFocusEventIfNeeded().

So I think we could just eliminate this whole "else" block and it will still
work.

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_manager_win.cc (right):

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_win.cc:141: if
(!node)
How about DCHECK(node)? This should never be called with null.

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_win.cc:176:
FireFocusEvent(active_descendant);
Same - I think this shouldn't be needed.

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_win.cc (left):

https://codereview.chromium.org/1899823002/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:3522: // Normally
focus events are handled elsewhere, however
Are you sure this isn't needed anymore? Do both NVDA and JAWS work correctly
with a <select multiple> with this change?

https://codereview.chromium.org/1899823002/diff/40001/content/test/data/acces...
File
content/test/data/accessibility/event/aria-combo-box-collapse-expected-mac.txt
(right):

https://codereview.chromium.org/1899823002/diff/40001/content/test/data/acces...
content/test/data/accessibility/event/aria-combo-box-collapse-expected-mac.txt:1:
AXFocusedUIElementChanged on AXList
I think if you get rid of the extra Focus event when we get an
ActiveDescendantChanged event, this will be eliminated.

https://codereview.chromium.org/1899823002/diff/40001/content/test/data/acces...
File
content/test/data/accessibility/event/aria-combo-box-expand-expected-mac.txt
(right):

https://codereview.chromium.org/1899823002/diff/40001/content/test/data/acces...
content/test/data/accessibility/event/aria-combo-box-expand-expected-mac.txt:6:
AXFocusedUIElementChanged on AXList
I don't see a focus event on the active descendant, should we see one?

[nektarios, 2016-04-29 15:06:02]

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

[commit-bot: I haz the power, 2016-04-29 15:06:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/60001

[commit-bot: I haz the power, 2016-04-29 15:20:48]

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

[commit-bot: I haz the power, 2016-04-29 15:20:49]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[nektarios, 2016-04-29 15:41:44]

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

[commit-bot: I haz the power, 2016-04-29 15:42:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/80001

[nektarios, 2016-04-29 15:43:38]

Description was changed from

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests
==========

to

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
3. Fires a focus event on the whole list or combo box when it collapses to make
Windows screen readers enable their virtual cursor again.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests
==========

[commit-bot: I haz the power, 2016-04-29 17:15:34]

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

[commit-bot: I haz the power, 2016-04-29 17:15:35]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dmazzoni, 2016-04-29 20:55:54]

lgtm once all tests pass

https://codereview.chromium.org/1899823002/diff/80001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_manager_mac.mm (right):

https://codereview.chromium.org/1899823002/diff/80001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_mac.mm:135: // For
combo boxes, focus should stay on the combo box so the user will not be
Consider doing this only for combo boxes where IsEditableText is true?

Sometimes people put role="combobox" on something that's more like a single-item
listbox and it maybe shouldn't behave the same.

https://codereview.chromium.org/1899823002/diff/80001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_manager_win.cc (right):

https://codereview.chromium.org/1899823002/diff/80001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_win.cc:142: // Combo
boxes have an invisible list inside them which tracks the active list
Isn't the combo box itself supposed to have activedescendant on it?

Either way, consider moving this to cross-platform code, because we should track
the activedescendant of a combobox on all platforms. If there's something
platform-specific, it should maybe just be a tiny override on Mac that says not
to follow active descendant focus on Mac because it doesn't work that way

https://codereview.chromium.org/1899823002/diff/80001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_manager_win.cc:144: if (node
&& node->GetRole() == ui::AX_ROLE_COMBO_BOX) {
Same here, only if combo box is editable?

[nektarios, 2016-05-24 16:42:07]

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

[commit-bot: I haz the power, 2016-05-24 16:42:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/100001

[commit-bot: I haz the power, 2016-05-24 17:11:36]

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

[commit-bot: I haz the power, 2016-05-24 17:11:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[nektarios, 2016-05-26 21:36:57]

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

[commit-bot: I haz the power, 2016-05-26 21:37:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/160001

[commit-bot: I haz the power, 2016-05-26 22:34:06]

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

[commit-bot: I haz the power, 2016-05-26 22:34:07]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[nektarios, 2016-05-27 00:13:45]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-27 00:13:46]

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/1899823002/#ps180001
(title: "Fixed Mac tests.")

[commit-bot: I haz the power, 2016-05-27 00:14:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/180001

[nektarios, 2016-05-27 00:14:44]

The CQ bit was unchecked by nektar@chromium.org

[nektarios, 2016-05-27 15:19:48]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-27 15:19:49]

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/1899823002/#ps220001
(title: "Disabled Mac test that crashes.")

[commit-bot: I haz the power, 2016-05-27 15:20:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/220001

[commit-bot: I haz the power, 2016-05-27 16:07:51]

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

[commit-bot: I haz the power, 2016-05-27 16:07:52]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[nektarios, 2016-05-27 16:44:43]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-27 16:44:44]

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/1899823002/#ps240001
(title: "Disabled another Mac test that was crashing.")

[commit-bot: I haz the power, 2016-05-27 16:45:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/240001

[commit-bot: I haz the power, 2016-05-27 18:04:06]

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

[commit-bot: I haz the power, 2016-05-27 18:04:08]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nektarios, 2016-05-27 18:54:52]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-27 18:54:53]

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/1899823002/#ps260001
(title: "Fixed WebKit tests.")

[commit-bot: I haz the power, 2016-05-27 18:55:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/260001

[commit-bot: I haz the power, 2016-05-27 20:43:45]

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

[commit-bot: I haz the power, 2016-05-27 20:43:47]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-05-28 22:14:55]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-05-28 22:15:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/260001

[nektarios, 2016-05-28 23:33:27]

The CQ bit was unchecked by nektar@chromium.org

[nektarios, 2016-05-28 23:34:58]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-28 23:34:59]

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/1899823002/#ps280001
(title: "Fixed menu-list-open Layout test.")

[commit-bot: I haz the power, 2016-05-28 23:35:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899823002/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899823002/280001

[commit-bot: I haz the power, 2016-05-29 01:26:40]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2016-05-29 01:28:17]

Description was changed from

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
3. Fires a focus event on the whole list or combo box when it collapses to make
Windows screen readers enable their virtual cursor again.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests
==========

to

==========
Uses the activedescendant_changed event received from Blink to fire the right
focus event on the browser side.

1. Changed all <select> based widgets (list box role and menu list popup role)
to fire activedescendant_changed events when the current selection changes. This
behavior is the same as with ARIA combo boxes and other ARIA-based widgets that
can have an active descendant. This means that we now have a uniform way of
notifying the browser when the active child of a widget has changed.
2. Modified platform-specific code to fire the focus events on the active
descendant when it changes, instead of relying on Blink.
3. Fires a focus event on the whole list or combo box when it collapses to make
Windows screen readers enable their virtual cursor again.
BUG=593646
R=dmazzoni@chromium.org
TESTED=Jaws NVDA and VoiceOver, browser tests

Committed: https://crrev.com/2c4ec4bed7e28e4f45efe742833dd3a154ac266b
Cr-Commit-Position: refs/heads/master@{#396653}
==========

[commit-bot: I haz the power, 2016-05-29 01:28:18]

Patchset 15 (id:??) landed as
https://crrev.com/2c4ec4bed7e28e4f45efe742833dd3a154ac266b
Cr-Commit-Position: refs/heads/master@{#396653}

[Guido Urdaneta, 2016-05-31 13:22:14]

A revert of this CL (patchset #15 id:280001) has been created in
https://codereview.chromium.org/2024053003/ by guidou@chromium.org.

The reason for reverting is: After this CL,
DumpAccessibilityEventsTest.AccessibilityEventsAriaComboBoxFocus started to fail
consistently on the Mac 10.11 Tests bot.
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/1737.

[chromium-reviews, 2016-05-31 15:19:53]

Better to only disable the test.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.