Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2069543004:
Adjust output rules related menus (Closed)
|
Created:
4 years, 6 months ago by David Tseng Modified:
4 years, 6 months ago Reviewers:
dmazzoni CC:
chromium-reviews, oshima+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, arv+watch_chromium.org, dtseng+watch_chromium.org, dmazzoni+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdjust output rules related menus
Fixes for Google Docs' menu:
1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon
2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node
3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements.
BUG=619950
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/ba8ead8fec469e986b2bf246398775d39bfca962
Cr-Commit-Position: refs/heads/master@{#400218}
Patch Set 1 #
Total comments: 2
Patch Set 2 : m #
Messages
Total messages: 24 (10 generated)
Description was changed from ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. ========== to ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. Make two fixes for menus in Google Docs: 1. the initial menu via alt+f wasn't being read. This was because focus was being set on the menu bar with an active descendant pointing to a menu item. We were previously only observing active descendant on the activeDescendantChanged event. 2. adjust the output rule for menuItem and menuItemCheckbox. For the former, don't play the button earcon (it gets quite noisy when arrowing through menus); for the latter, include the checkbox state and earcon. TEST=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
dtseng@chromium.org changed reviewers: + dmazzoni@chromium.org
Description was changed from ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. Make two fixes for menus in Google Docs: 1. the initial menu via alt+f wasn't being read. This was because focus was being set on the menu bar with an active descendant pointing to a menu item. We were previously only observing active descendant on the activeDescendantChanged event. 2. adjust the output rule for menuItem and menuItemCheckbox. For the former, don't play the button earcon (it gets quite noisy when arrowing through menus); for the latter, include the checkbox state and earcon. TEST=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. Make two fixes for menus in Google Docs: 1. the initial menu via alt+f wasn't being read. This was because focus was being set on the menu bar with an active descendant pointing to a menu item. We were previously only observing active descendant on the activeDescendantChanged event. 2. adjust the output rule for menuItem and menuItemCheckbox. For the former, don't play the button earcon (it gets quite noisy when arrowing through menus); for the latter, include the checkbox state and earcon. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js (left): https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js:234: earconId: 'BUTTON' Did you mean to get rid of the earcon for menuItemCheckBox, below?
https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js (left): https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js:234: earconId: 'BUTTON' On 2016/06/14 21:46:32, dmazzoni wrote: > Did you mean to get rid of the earcon for menuItemCheckBox, below? I did; not sure how you feel about it, but output becomes pretty busy with both checbox and button earcons. I found the button earcon in menus to be a bit much in general. Alternatively could try to limit the playing of earcons simultaneously.
On 2016/06/15 22:59:21, David Tseng wrote: > https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... > File chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js > (left): > > https://codereview.chromium.org/2069543004/diff/1/chrome/browser/resources/ch... > chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js:234: > earconId: 'BUTTON' > On 2016/06/14 21:46:32, dmazzoni wrote: > > Did you mean to get rid of the earcon for menuItemCheckBox, below? > > I did; not sure how you feel about it, but output becomes pretty busy with both > checbox and button earcons. I found the button earcon in menus to be a bit much > in general. > Alternatively could try to limit the playing of earcons simultaneously. I agree there shouldn't be both, I think we should have just a checkbox earcon OR a button earcon. I think some earcon for each menu item is good. It looks like your change removes the earcon for a normal menu item.
To clarify, I think you deleted the earcon for menuItem, but I think you meant to delete the earcon for menuItemCheckbox
On Wed, Jun 15, 2016 at 4:05 PM, <dmazzoni@chromium.org> wrote: > To clarify, I think you deleted the earcon for menuItem, but I think you > meant > to delete the earcon for menuItemCheckbox > > Ah, yes. I think I wanted to delete both. Any objections? > https://codereview.chromium.org/2069543004/ > -- 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.
lgtm. Don't forget menuitemradio, make it consistent with menuitemcheckbox. On Wed, Jun 15, 2016 at 4:24 PM David Tseng <dtseng@chromium.org> wrote: > On Wed, Jun 15, 2016 at 4:05 PM, <dmazzoni@chromium.org> wrote: > >> To clarify, I think you deleted the earcon for menuItem, but I think you >> meant >> to delete the earcon for menuItemCheckbox >> >> Ah, yes. I think I wanted to delete both. Any objections? >> https://codereview.chromium.org/2069543004/ >> > > -- > 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. > -- 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.
Description was changed from ========== Adjust menuItemCheckbox output and always follow active descendant relations when outputing. Make two fixes for menus in Google Docs: 1. the initial menu via alt+f wasn't being read. This was because focus was being set on the menu bar with an active descendant pointing to a menu item. We were previously only observing active descendant on the activeDescendantChanged event. 2. adjust the output rule for menuItem and menuItemCheckbox. For the former, don't play the button earcon (it gets quite noisy when arrowing through menus); for the latter, include the checkbox state and earcon. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Adjust output rules related menus Fixes for Google Docs' menu: 1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon 2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node 3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
On 2016/06/15 23:43:07, dmazzoni wrote: > lgtm. Don't forget menuitemradio, make it consistent with menuitemcheckbox. Done. I also added a "ignoreAncestry" attribute to role info and check state earcons for menuItemCheckBox/menuItemRadio. See updated description for a little more detail. > > > On Wed, Jun 15, 2016 at 4:24 PM David Tseng <mailto:dtseng@chromium.org> wrote: > > > On Wed, Jun 15, 2016 at 4:05 PM, <mailto:dmazzoni@chromium.org> wrote: > > > >> To clarify, I think you deleted the earcon for menuItem, but I think you > >> meant > >> to delete the earcon for menuItemCheckbox > >> > >> Ah, yes. I think I wanted to delete both. Any objections? > >> https://codereview.chromium.org/2069543004/ > >> > > > > -- > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. > > > > -- > 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 mailto:chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2069543004/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
The CQ bit was checked by dmazzoni@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2069543004/20001
Message was sent while issue was closed.
Description was changed from ========== Adjust output rules related menus Fixes for Google Docs' menu: 1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon 2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node 3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Adjust output rules related menus Fixes for Google Docs' menu: 1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon 2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node 3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Adjust output rules related menus Fixes for Google Docs' menu: 1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon 2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node 3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Adjust output rules related menus Fixes for Google Docs' menu: 1. menuItem, menuItemCheckBox, and menuItemRadio don't play a button earcon 2. menuItem, menuItemCheckBox, and menuItemRadio all ignore output of their ancestry. Their ancestry gets created/destroyed on each key press, so the ancestry diffs are always up to the application node 3. play checked/unchecked earcons for menuItemCheckBox and menuItemRadio. Also, add specific output rules for these; they cannot inherit checkBox and radioButton rules because they must include index/total announcements. BUG=619950 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/ba8ead8fec469e986b2bf246398775d39bfca962 Cr-Commit-Position: refs/heads/master@{#400218} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ba8ead8fec469e986b2bf246398775d39bfca962 Cr-Commit-Position: refs/heads/master@{#400218} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
