[Views] Make the permissions menus on the Website Settings UI focusable by the TAB key and provuide…

[Views] Make the permissions menus on the Website Settings UI focusable by the TAB key and provuide accasability information.


BUG=151589
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158295

[markusheintz_, 2012-09-21 18:41:53]

Mike could you please review this CL? Thanks a lot.

[msw, 2012-09-21 22:32:46]

Sorry for the delay; just a nit and a question.

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
File chrome/browser/ui/views/website_settings/permission_selector_view.cc
(right):

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
chrome/browser/ui/views/website_settings/permission_selector_view.cc:319:
menu_button_->set_focusable(true);
nit: s/true/button_enabled/ ?

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
chrome/browser/ui/views/website_settings/permission_selector_view.cc:320:
menu_button_->SetAccessibleName(
How are this AccessibleName and the AccessibleViewState::value strings used, and
why should they differ?

[tfarina, 2012-09-21 22:34:38]

Copying Dominic for accessibility!

[dmazzoni, 2012-09-21 22:45:19]

lgtm for accessibility

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
File chrome/browser/ui/views/website_settings/permission_selector_view.cc
(right):

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
chrome/browser/ui/views/website_settings/permission_selector_view.cc:320:
menu_button_->SetAccessibleName(
On 2012/09/21 22:32:46, msw wrote:
> How are this AccessibleName and the AccessibleViewState::value strings used,
and
> why should they differ?

I think this is right. If this is a multiple-choice control, then the name is
the description of what the control is for, and the value is the
currently-selected option.

[msw, 2012-09-22 00:56:08]

Thanks Dominic! LGTM w/ set_focusable(button_enabled) nit.

[markusheintz_, 2012-09-24 14:08:31]

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
File chrome/browser/ui/views/website_settings/permission_selector_view.cc
(right):

https://codereview.chromium.org/10961048/diff/1/chrome/browser/ui/views/websi...
chrome/browser/ui/views/website_settings/permission_selector_view.cc:319:
menu_button_->set_focusable(true);
On 2012/09/21 22:32:46, msw wrote:
> nit: s/true/button_enabled/ ?

Done. It actually doesn't matter since the is focusable method "returns
(enabled_ && focusabble_). But I think it is good that you pointed this out
since using |button_enabled_| instead of true is probably more robust.

[commit-bot: I haz the power, 2012-09-24 14:53:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/markusheintz@chromium.org/10961048/1003

[commit-bot: I haz the power, 2012-09-24 17:01:34]

Change committed as 158295