The default bug view has changed. See this FAQ.

Implement initial canvas accessible and baseline test.

RESOLVED FIXED in mozilla11

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: davidb, Assigned: davidb)

Tracking

unspecified
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Create a canvas html accessible, and a baseline test.
(Assignee)

Comment 1

5 years ago
Created attachment 576815 [details] [diff] [review]
patch
Attachment #576815 - Flags: review?(surkov.alexander)

Comment 2

5 years ago
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
Attachment #576815 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 576937 [details] [diff] [review]
just the layout bit.
Attachment #576937 - Flags: review?(ehsan)
Comment on attachment 576937 [details] [diff] [review]
just the layout bit.

r=me
Attachment #576937 - Flags: review?(ehsan) → review+
(Assignee)

Comment 5

5 years ago
Thanks guys.

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d6a7336070
https://hg.mozilla.org/mozilla-central/rev/39d6a7336070
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.