705179 - Implement initial canvas accessible and baseline test.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Create a canvas html accessible, and a baseline test.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 576815 [details] [diff] [review]
patch

Review of attachment 576815 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/nsAccessibilityService.h
@@ +94,5 @@
>    virtual already_AddRefed<nsAccessible>
>      CreateHTMLHRAccessible(nsIContent* aContent, nsIPresShell* aPresShell);
>    virtual already_AddRefed<nsAccessible>
> +    CreateHTMLCanvasAccessible(nsIContent* aContent, nsIPresShell* aPresShell);
> +  virtual already_AddRefed<nsAccessible>

nit: it doesn't need to be virtual, put it in alphabetical order

::: accessible/src/html/nsHTMLCanvasAccessible.cpp
@@ +18,5 @@
> + * Mozilla Foundation
> + * Portions created by the Initial Developer are Copyright (C) 2011
> + * the Initial Developer. All Rights Reserved.
> + *
> + * Contributor(s):

nit: put your name here

@@ +44,5 @@
> +  nsHyperTextAccessible(aContent, aShell)
> +{
> +}
> +
> +NS_IMPL_ISUPPORTS_INHERITED0(nsHTMLCanvasAccessible, nsHyperTextAccessible)

It doesn't sound like we really need this because nsISupports::QueryInterface takes const nsIID&, IUnknown version takes nsIID&. nsQueryInterface class deals with const nsIID& so it doesn't hit IUnknown::QueryInterface.

@@ +52,5 @@
> +{
> +  return nsIAccessibleRole::ROLE_CANVAS;
> +}
> +
> +

nit: extra empty line

::: accessible/src/html/nsHTMLCanvasAccessible.h
@@ +18,5 @@
> + * Mozilla Foundation
> + * Portions created by the Initial Developer are Copyright (C) 2011
> + * the Initial Developer. All Rights Reserved.
> + *
> + * Contributor(s):

nit: your name

@@ +39,5 @@
> +
> +#ifndef _nsHTMLCanvasAccessible_H_
> +#define _nsHTMLCanvasAccessible_H_
> +
> +class nsHTMLCanvasAccessible : public nsHyperTextAccessible

nit: add a comment to describe the class

@@ +42,5 @@
> +
> +class nsHTMLCanvasAccessible : public nsHyperTextAccessible
> +{
> +public:
> +  nsHTMLCanvasAccessible(nsIContent *aContent, nsIWeakReference *aShell);

nit: do type*, not type *

@@ +45,5 @@
> +public:
> +  nsHTMLCanvasAccessible(nsIContent *aContent, nsIWeakReference *aShell);
> +  virtual ~nsHTMLCanvasAccessible() { }
> +
> +  NS_DECL_ISUPPORTS_INHERITED

you don't need this

@@ +51,5 @@
> +  // nsAccessible
> +  virtual PRUint32 NativeRole();
> +};
> +
> +

nit: extra empty line

::: layout/generic/nsHTMLCanvasFrame.cpp
@@ +366,5 @@
>  {
> +  nsAccessibilityService* accService = nsIPresShell::AccService();
> +  if (accService) {
> +    return accService->CreateHTMLCanvasAccessible(mContent,
> +                                                 PresContext()->PresShell());

nit: indent properly second line

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: just the layout bit.

Comment 4

[(no longer active)]

Comment on attachment 576937 [details] [diff] [review]
just the layout bit.

r=me

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks guys.

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d6a7336070

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/39d6a7336070