Issue 3250014: Update browser accessibility tree on a renderer accessibility object state ch...

Issue 3250014: Update browser accessibility tree on a renderer accessibility object state ch... (Closed)

Created:
10 years, 3 months ago by Chris Guillory

Modified:
9 years, 7 months ago

Reviewers:
dmazzoni

CC:
chromium-reviews, brettw-cc_chromium.org, Paweł Hajdan Jr., darin-cc_chromium.org, ben+cc_chromium.org, David Tseng, dglazkov

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

Visibility:
Public.

Description

Update browser accessibility tree on a renderer accessibility object state change. BUG=13291 TEST=interactive_ui_tests AccessibilityWinBrowserTest.TestDynamicAccessibilityTree R=dmazzoni Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58194

Patch Set 1 #

Patch Set 2 : Fix lint errors #

Total comments: 8

Created: 10 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+157 lines, -64 lines)			Patch
M	chrome/browser/accessibility_win_browsertest.cc	View		9 chunks	+65 lines, -6 lines	2 comments	Download
M	chrome/browser/browser_accessibility_manager_win.h	View	1	1 chunk	+8 lines, -1 line	0 comments	Download
M	chrome/browser/browser_accessibility_manager_win.cc	View		3 chunks	+47 lines, -20 lines	4 comments	Download
M	chrome/browser/browser_accessibility_win.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/browser_accessibility_win.cc	View		2 chunks	+6 lines, -0 lines	0 comments	Download
M	chrome/browser/renderer_host/render_view_host.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_view_host.cc	View		1 chunk	+8 lines, -2 lines	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_win.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_win.cc	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	chrome/common/render_messages_internal.h	View		1 chunk	+3 lines, -5 lines	2 comments	Download
M	chrome/renderer/render_view.cc	View		3 chunks	+8 lines, -24 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

dmazzoni

LGTM. http://codereview.chromium.org/3250014/diff/4002/15001 File chrome/browser/accessibility_win_browsertest.cc (right): http://codereview.chromium.org/3250014/diff/4002/15001#newcode345 chrome/browser/accessibility_win_browsertest.cc:345: AccessibleChecker div_checker1(L"", L"div", L""); How about div1_checker through ...

10 years, 3 months ago (2010-09-01 15:10:52 UTC) #3

Chris Guillory

10 years, 3 months ago (2010-09-01 15:35:26 UTC) #4

http://codereview.chromium.org/3250014/diff/4002/15001
File chrome/browser/accessibility_win_browsertest.cc (right):

http://codereview.chromium.org/3250014/diff/4002/15001#newcode345
chrome/browser/accessibility_win_browsertest.cc:345: AccessibleChecker
div_checker1(L"", L"div", L"");
On 2010/09/01 15:10:52, Dominic Mazzoni wrote:
> How about div1_checker through div3_checker?

Done. I like that better.

http://codereview.chromium.org/3250014/diff/4002/15002
File chrome/browser/browser_accessibility_manager_win.cc (right):

http://codereview.chromium.org/3250014/diff/4002/15002#newcode161
chrome/browser/browser_accessibility_manager_win.cc:161: this,
On 2010/09/01 15:10:52, Dominic Mazzoni wrote:
> nit: Indent 4 spaces

Done.

http://codereview.chromium.org/3250014/diff/4002/15002#newcode177
chrome/browser/browser_accessibility_manager_win.cc:177: if
(focus_->IsDescendantOf(old_browser_acc)) {
On 2010/09/01 15:10:52, Dominic Mazzoni wrote:
> We definitely need to fix the focus (it can't point to an invalid node), but
> should we be setting it to this parent node?
> 
> Maybe the state change notification from the renderer to the browser should
pass
> along the (new id) of the focused node?
> 
That's a good thought. I remove this for now as this portion isn't tested
currently.

http://codereview.chromium.org/3250014/diff/4002/15011
File chrome/common/render_messages_internal.h (right):

http://codereview.chromium.org/3250014/diff/4002/15011#newcode2224
chrome/common/render_messages_internal.h:2224: // Sent by the render when the
state of an accessibility node changes.
On 2010/09/01 15:10:52, Dominic Mazzoni wrote:
> render -> renderer?

Done.

Expand Messages | Collapse Messages