Makes Chrome's app menu have a vertical orientation.

Makes Chrome's app menu have a vertical orientation.

Required by a future update to Jaws that will indicate the presence of submenus properly, e.g. "History, submenu", instead of only "History".
BUG=635614
R=dmazzoni@chromium.org
TESTED=Looking at the IA2States in NVDA's Log Viewer

Committed: https://crrev.com/5bc8521246373a8f78dd807a47459051369fad1d
Cr-Commit-Position: refs/heads/master@{#425477}

[dmazzoni, 2016-10-13 18:03:37]

I want to fix this but the fix makes no sense to me. The menu bar is not
vertical, so I don't see why we should use that state.

Semantically I think what we really have is a menu button in a toolbar.

Perhaps the parent of the MENUITEM should have a role of MENU with a state of
VERTICAL, would that help?

To better describe: Chrome has a toolbar that's horizontal. The rightmost item
on the toolbar is a single menu button. Pressing the menu button opens a
vertical menu.

[nektarios, 2016-10-14 15:40:26]

Your recommendation has been implemented. All sub-menus are now marked as
vertical.

[nektarios, 2016-10-14 15:41:23]

Also tested with NVDA's Log Viewer and the vertical IA2 state is applied
correctly.

[dmazzoni, 2016-10-14 15:45:01]

dmazzoni@chromium.org changed reviewers:
+ sky@chromium.org

[dmazzoni, 2016-10-14 15:45:02]

lgtm

+sky for owners review

[sky, 2016-10-14 16:48:42]

It wasn't readily clear what 'vertical' means in this context. I looked at the
header and there isn't a single comment as to what 'AX_STATE_VERTICAL' is (
https://cs.chromium.org/chromium/src/out/Debug/gen/ui/accessibility/ax_enums....
). It may be obvious to you guys, but I suspect not many will know. Could you
please add a description so I know what it's applicable to submenuview?

[nektarios, 2016-10-14 19:19:42]

Vertical orientation is defined in the ARIA 1.1 Spec here:
https://www.w3.org/TR/wai-aria-1.1/#aria-orientation
In short, all sub-menus expand from top to bottom when you add more items in
them, so they are oriented vertically. Most toolbars are oriented horizontally
because the more items you add to them they either grow on the left or on the
right.
This can mean two things:
1. It is more likely that cursor up and down will be used for menus and cursor
left and right for navigating toolbars respectively.
2. If an element is collapsed, e.g. a combo box or a divider, you would know on
which direction it will expand on when expanded.
For example, Voiceover might say "Vertical divider, collapsed on left".

[sky, 2016-10-14 19:21:35]

LGTM - please document the enums. Having to know what w3c spec to find and where
to look is tedious.

[dmazzoni, 2016-10-14 19:24:57]

Nektarios, please just add a concise description to
ax_enums.idl, something like:

...
// Collection of items like a list, menu has a vertical orientation
// (as opposed to horizontal).
vertical,
...

[chromium-reviews, 2016-10-14 19:32:34]

Oh sorry, I didn't say that I already added a comment in the submenu 
class itself.
I added two more in AXEnums.idl

-- 
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-10-14 19:37:20]

https://codereview.chromium.org/2418523004/diff/60001/ui/accessibility/ax_enu...
File ui/accessibility/ax_enums.idl (right):

https://codereview.chromium.org/2418523004/diff/60001/ui/accessibility/ax_enu...
ui/accessibility/ax_enums.idl:213: horizontal,  // Grows horizontally, e.g. most
toolbars and separators.
The code generator doesn't work with comments at the end of a line.
Please put the comments on the previous line.

[chromium-reviews, 2016-10-14 19:44:24]

Comments moved.


-- 
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-10-14 20:49:17]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-10-14 20:49:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2418523004/#ps80001
(title: "Moved clarifying comments to a previous line.")

[commit-bot: I haz the power, 2016-10-14 20:49:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 21:43:53]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-14 21:45:54]

Description was changed from

==========
Makes Chrome's app menu have a vertical orientation.

Required by a future update to Jaws that will indicate the presence of submenus
properly, e.g. "History, submenu", instead of only "History".
BUG=635614
R=dmazzoni@chromium.org
TESTED=Looking at the IA2States in NVDA's Log Viewer
==========

to

==========
Makes Chrome's app menu have a vertical orientation.

Required by a future update to Jaws that will indicate the presence of submenus
properly, e.g. "History, submenu", instead of only "History".
BUG=635614
R=dmazzoni@chromium.org
TESTED=Looking at the IA2States in NVDA's Log Viewer

Committed: https://crrev.com/5bc8521246373a8f78dd807a47459051369fad1d
Cr-Commit-Position: refs/heads/master@{#425477}
==========

[commit-bot: I haz the power, 2016-10-14 21:45:56]

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