Modified DialogDelegateView to be accessible

Modified DialogDelegateView to be accessible

Added GetAccessibleState to DialogDelegateView to return the proper
accessibility state, and added a call to NotifyAccessibilityEvent which
fires when the dialog opens.
Implementation details on comment #6 on bug.

BUG=425311
Committed: https://crrev.com/2156bd153179e615470d9ca3a70a920440999a37
Cr-Commit-Position: refs/heads/master@{#302215}

[sashab, 2014-10-30 04:28:55]

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

[sashab, 2014-10-30 04:28:55]

Small patch for adding proper accessibility support to DialogDelegateViews. This
was explained to me by the accessibility team - see comment #6 on bug.

[sky, 2014-10-30 13:13:28]

https://codereview.chromium.org/691703002/diff/1/ui/views/window/dialog_deleg...
File ui/views/window/dialog_delegate.h (right):

https://codereview.chromium.org/691703002/diff/1/ui/views/window/dialog_deleg...
ui/views/window/dialog_delegate.h:140: // Overridden from View:
I don't think either of these should be private. Subclasses may want to
override. In fact you need to investigate all subclasses and make sure if one is
overriding they are invoking DialogDelegateView::Foo.

[sashab, 2014-10-30 23:12:03]

Should I TBR these authors, given that this is a refactoring-style change?

https://codereview.chromium.org/691703002/diff/1/ui/views/window/dialog_deleg...
File ui/views/window/dialog_delegate.h (right):

https://codereview.chromium.org/691703002/diff/1/ui/views/window/dialog_deleg...
ui/views/window/dialog_delegate.h:140: // Overridden from View:
On 2014/10/30 13:13:28, sky wrote:
> I don't think either of these should be private. Subclasses may want to
> override. In fact you need to investigate all subclasses and make sure if one
is
> overriding they are invoking DialogDelegateView::Foo.

Looking at all the direct subclasses, the following override
ViewHierarchyChanged:
- NetworkConfigView
- CollectedCookieView
- ExtensionInstallDialogView
- HungRendererView
- ProfileSigninConfirmationDialogViews
- SignedCertificateTimestampsViews
- TaskManagerView
I have updated these to call the parent ViewHierarchyChanged. I don't know an
easy way to search through all their subclasses recursively... to find all
classes that override ViewHierarchyChanged that also indirectly inherit from
DialogDelegateView.

The following override GetAccessibleState:
- NetworkConfigView
- BookmarkEditorView
However, I've left them as they are since they override GetWindowTitle() to use
a different title (they want to display one title but have a different one for
accessibility purposes). Made them call the parent class just to remove the
repeated 'role = AX_ROLE_DIALOG' code.

[sky, 2014-10-31 02:33:05]

LGTM

https://codereview.chromium.org/691703002/diff/40001/ui/views/window/dialog_d...
File ui/views/window/dialog_delegate.cc (right):

https://codereview.chromium.org/691703002/diff/40001/ui/views/window/dialog_d...
ui/views/window/dialog_delegate.cc:249: if (details.is_add && details.child ==
this)
I think you should verify you're in a widget here too, eg GetWidget() returns
non-null.

[sashab, 2014-10-31 02:56:47]

https://codereview.chromium.org/691703002/diff/40001/ui/views/window/dialog_d...
File ui/views/window/dialog_delegate.cc (right):

https://codereview.chromium.org/691703002/diff/40001/ui/views/window/dialog_d...
ui/views/window/dialog_delegate.cc:249: if (details.is_add && details.child ==
this)
On 2014/10/31 02:33:05, sky wrote:
> I think you should verify you're in a widget here too, eg GetWidget() returns
> non-null.

Good call. Added to the if-statement.

[sashab, 2014-10-31 02:56:52]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-10-31 02:58:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/691703002/60001

[commit-bot: I haz the power, 2014-10-31 03:39:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-31 03:40:18]

Patchset 4 (id:??) landed as
https://crrev.com/2156bd153179e615470d9ca3a70a920440999a37
Cr-Commit-Position: refs/heads/master@{#302215}