739883 - decomtaminate impl of GetSummary() on accessible tables

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

see procedure in bug 739882 accept this time with GetSummary()

Comment 1

[Max Li [:maxli]]

Attachment: Patch (v1)

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 615206 [details] [diff] [review]
Patch (v1)

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

f=me with comments addressed, requesting r from Trevor

::: accessible/src/generic/ARIAGridAccessible.h
@@ +65,5 @@
>    // nsIAccessibleTable
>    NS_DECL_OR_FORWARD_NSIACCESSIBLETABLE_WITH_XPCACCESSIBLETABLE
>  
> +  // TableAccessible
> +  virtual void Summary(nsString& aSummary);

you don't need to override it since it does the same as TableAccessible::Summary

::: accessible/src/html/nsHTMLTableAccessible.cpp
@@ +561,2 @@
>  
> +  table->GetSummary(aSummary);

I'm not sure whether things like <div style="display: table"> implement nsIDOMHTMLTableElement interface, it's safer to have null check like
if (table)
  table->GetSummary(aSummary);

::: accessible/src/xul/nsXULListboxAccessible.h
@@ +104,5 @@
>    // nsIAccessibleTable
>    NS_DECL_OR_FORWARD_NSIACCESSIBLETABLE_WITH_XPCACCESSIBLETABLE
>  
> +  // TableAccessible
> +  virtual void Summary(nsString& aSummary);

you don't need it as well

::: accessible/src/xul/nsXULTreeGridAccessible.h
@@ +60,5 @@
>    // nsIAccessibleTable
>    NS_DECL_OR_FORWARD_NSIACCESSIBLETABLE_WITH_XPCACCESSIBLETABLE
>  
> +  // TableAccessible
> +  virtual void Summary(nsString& aSummary);

same

Comment 3

[Max Li [:maxli]]

Attachment: Patch v2

Comments addressed

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 615231 [details] [diff] [review]
Patch v2

>+  nsAutoString summary(aSummary);
>+  mTable->Summary(summary);

don't pass anything to the nsAutoString constructor.

r=me with that

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 615231 [details] [diff] [review]
Patch v2


>+nsHTMLTableAccessible::Summary(nsString& aSummary)
> {
>   nsCOMPtr<nsIDOMHTMLTableElement> table(do_QueryInterface(mContent));
>-  NS_ENSURE_TRUE(table, NS_ERROR_FAILURE);
>-
>-  return table->GetSummary(aSummary);
>+  

empty line should be empty (no spaces)

I'll fix nits before landing

Comment 6

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3302067526f6

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/3302067526f6