Issue 174252: Adds support and exposure of additional accessibility roles. Includes naming ...

Issue 174252: Adds support and exposure of additional accessibility roles. Includes naming ... (Closed)

Created:
11 years, 4 months ago by Jonas Klink (Google)

Modified:
9 years, 7 months ago

Reviewers:
sky

CC:
chromium-reviews_googlegroups.com

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Adds support and exposure of additional accessibility roles. Includes naming of the LocationBar, and correctly exposing MSAA/ARIA roles for Documents, Graphics, Menubars and Toolbars. BUG=13291, 19982 TEST=Assign @role menubar or toolbar to any dom element, and use Inspect32 (or similar tool) to see it exposed correctly. In the same way, <html> tag is exposed as role document. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=24049

Patch Set 1 #

Total comments: 3

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+86 lines, -5 lines)			Patch
M	chrome/browser/browser_accessibility.cc	View	3 chunks	+6 lines, -0 lines	0 comments	Download
M	chrome/browser/views/browser_views_accessibility_browsertest.cc	View	2 chunks	+26 lines, -0 lines	0 comments	Download
M	chrome/browser/views/location_bar_view.h	View	2 chunks	+6 lines, -3 lines	1 comment	Download
M	chrome/browser/views/location_bar_view.cc	View	2 chunks	+14 lines, -0 lines	1 comment	Download
M	chrome/browser/views/toolbar_view.cc	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	views/accessibility/accessibility_types.h	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	views/accessibility/view_accessibility.cc	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	views/controls/image_view.h	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	views/controls/image_view.cc	View	2 chunks	+18 lines, -1 line	1 comment	Download
M	webkit/glue/glue_accessibility_object.cc	View	3 chunks	+6 lines, -0 lines	0 comments	Download
M	webkit/glue/webaccessibility.h	View	2 chunks	+4 lines, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

sky

Just nits. LGTM otherwise http://codereview.chromium.org/174252/diff/1/6 File chrome/browser/views/location_bar_view.cc (right): http://codereview.chromium.org/174252/diff/1/6#newcode706 Line 706: (*name).assign(accessible_name_); name->assign http://codereview.chromium.org/174252/diff/1/5 File ...

11 years, 4 months ago (2009-08-21 21:33:39 UTC) #2

klink_google.com

11 years, 4 months ago (2009-08-21 22:28:34 UTC) #3

On Fri, Aug 21, 2009 at 2:33 PM, <sky@chromium.org> wrote:

> Just nits. LGTM otherwise
>
>
> http://codereview.chromium.org/174252/diff/1/6
> File chrome/browser/views/location_bar_view.cc (right):
>
> http://codereview.chromium.org/174252/diff/1/6#newcode706
> Line 706: (*name).assign(accessible_name_);
> name->assign


Fixed.


>
>
> http://codereview.chromium.org/174252/diff/1/5
> File chrome/browser/views/location_bar_view.h (right):
>
> http://codereview.chromium.org/174252/diff/1/5#newcode516
> Line 516: };
> Can you add DISALLOW_COPY_AND_ASSIGN while you're here.


Done.


>
>
> http://codereview.chromium.org/174252/diff/1/3
> File views/controls/image_view.cc (right):
>
> http://codereview.chromium.org/174252/diff/1/3#newcode143
> Line 143: *name = GetTooltipText();
> Both of these should be consistent. Either use tooltip_text_ in both, or
> GetTooltipText in both. Would also be nice if the format of this matched
> GetAccessibleRole, eg:
>
> if (!tooltip_text_.empty())
>  return false;
> ...


Changed. Removed the unnecessary else.


>
>
> http://codereview.chromium.org/174252
>

Expand Messages | Collapse Messages