Fix volume slider to emit value changed events and do not focus system tray item for transient views

Fix volume slider to emit value changed events and do not focus system tray item for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The former has some logic to detect when ChromeVox is on. If so, it tries to focus the first item. This is problematic because PopupDetailedView is transient and disappears.

The general bug is that ChromeVox gets the focus event and flips itself to compat mode. However, true focus still lives in the web content.

BUG=559339
Committed: https://crrev.com/b6cac2bcff25657f70fcd69f414be4884675a443
Cr-Commit-Position: refs/heads/master@{#365565}

[David Tseng, 2015-12-15 21:00:37]

Description was changed from

==========
Fix Volume
==========

to

==========
Fix volume slider to emit value changed events and do not focus system tray item
for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The
former has some logic to detect when ChromeVox is on. If so, it tries to focus
the first item. This is problematic because PopupDetailedView is transient and
disappears.

The general bug is that ChromeVox gets the focus event and flips itself to
compat mode. However, true focus still lives in the web content.

BUG=559339
==========

[David Tseng, 2015-12-15 21:00:37]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org, oshima@chromium.org

[David Tseng, 2015-12-15 21:01:41]

oshima for ash. In particular, if there are any negative consequences of
skipping focusing if needed of system tray item.
dmazzoni for ChromeVox.

[oshima, 2015-12-15 23:52:39]

ash lgtm with a nit

https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
File ash/system/tray/system_tray.cc (right):

https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
ash/system/tray/system_tray.cc:105: !is_persistent) {
Am I correct that we do not want to set the focus for solo volume/brightness
bubble so that it won't trigger the spoken feedback? 

can you update the comment?

[oshima, 2015-12-15 23:53:01]

On 2015/12/15 23:52:39, oshima wrote:
> ash lgtm with a nit
> 
>
https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
> File ash/system/tray/system_tray.cc (right):
> 
>
https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
> ash/system/tray/system_tray.cc:105: !is_persistent) {
> Am I correct that we do not want to set the focus for solo volume/brightness
> bubble so that it won't trigger the spoken feedback? 
> 
> can you update the comment?

And  can you look into test failures?

[David Tseng, 2015-12-16 16:47:49]

Test failure fixed as well. The failure was uncovered by my cl and I filed:
https://code.google.com/p/chromium/issues/detail?id=570341

to track it.

https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
File ash/system/tray/system_tray.cc (right):

https://codereview.chromium.org/1523953004/diff/20001/ash/system/tray/system_...
ash/system/tray/system_tray.cc:105: !is_persistent) {
On 2015/12/15 23:52:39, oshima wrote:
> Am I correct that we do not want to set the focus for solo volume/brightness
> bubble so that it won't trigger the spoken feedback? 
> 
> can you update the comment?

That's correct. The semantics are important here. We want to get the
ui::AX_EVENT_VALUE_CHANGED events emitted by the slider, but not the focus
events.

Comment revised.

[David Tseng, 2015-12-16 17:21:46]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2015-12-16 17:21:46]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/1523953004/#ps60001
(title: "Revise comment")

[commit-bot: I haz the power, 2015-12-16 17:22:39]

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

[commit-bot: I haz the power, 2015-12-16 18:25:22]

Description was changed from

==========
Fix volume slider to emit value changed events and do not focus system tray item
for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The
former has some logic to detect when ChromeVox is on. If so, it tries to focus
the first item. This is problematic because PopupDetailedView is transient and
disappears.

The general bug is that ChromeVox gets the focus event and flips itself to
compat mode. However, true focus still lives in the web content.

BUG=559339
==========

to

==========
Fix volume slider to emit value changed events and do not focus system tray item
for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The
former has some logic to detect when ChromeVox is on. If so, it tries to focus
the first item. This is problematic because PopupDetailedView is transient and
disappears.

The general bug is that ChromeVox gets the focus event and flips itself to
compat mode. However, true focus still lives in the web content.

BUG=559339
==========

[commit-bot: I haz the power, 2015-12-16 18:25:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-16 18:26:46]

Description was changed from

==========
Fix volume slider to emit value changed events and do not focus system tray item
for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The
former has some logic to detect when ChromeVox is on. If so, it tries to focus
the first item. This is problematic because PopupDetailedView is transient and
disappears.

The general bug is that ChromeVox gets the focus event and flips itself to
compat mode. However, true focus still lives in the web content.

BUG=559339
==========

to

==========
Fix volume slider to emit value changed events and do not focus system tray item
for transient views.

SystemTray::ShowItems gets called by SystemTrayItem::PopupDetailedView. The
former has some logic to detect when ChromeVox is on. If so, it tries to focus
the first item. This is problematic because PopupDetailedView is transient and
disappears.

The general bug is that ChromeVox gets the focus event and flips itself to
compat mode. However, true focus still lives in the web content.

BUG=559339

Committed: https://crrev.com/b6cac2bcff25657f70fcd69f414be4884675a443
Cr-Commit-Position: refs/heads/master@{#365565}
==========

[commit-bot: I haz the power, 2015-12-16 18:26:47]

Patchset 4 (id:??) landed as
https://crrev.com/b6cac2bcff25657f70fcd69f414be4884675a443
Cr-Commit-Position: refs/heads/master@{#365565}