636945 - add downcasting for nsRootAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 1

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

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

>-  already_AddRefed<nsRootAccessible> GetRootAccessible();
>+  nsRootAccessible* RootAccessible() const;

Are you sure you prefer the new name?

>+  inline bool IsRoot() const { return mFlags & eRootAccessible; }
>+  nsRootAccessible* AsRoot();

I'd prefer IsRootAccessible, and AsRootAccessible, do you agree it is more clear?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> Comment on attachment 515290 [details] [diff] [review]
> patch
> 
> >-  already_AddRefed<nsRootAccessible> GetRootAccessible();
> >+  nsRootAccessible* RootAccessible() const;
> 
> Are you sure you prefer the new name?

in general we don't use Get prefix for getter methods. There are many examples in new code.

> >+  inline bool IsRoot() const { return mFlags & eRootAccessible; }
> >+  nsRootAccessible* AsRoot();
> 
> I'd prefer IsRootAccessible, and AsRootAccessible, do you agree it is more
> clear?

it's more clear, but other Is and As methods don't include Accessible word. I thought about RootDoc but it may confuse with nsRootAccessible class name.

Comment 3

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

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

This should be low risk, but I don't see why we'd want/need to land it now.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)

> This should be low risk, but I don't see why we'd want/need to land it now.

The bug 636945 is based on top on this. I just removed bad things while it safe in terms of risk estimation. I think it's ok to take it to avoid unperf by default things introduction.

Comment 5

[alexander :surkov (:asurkov)]

perhaps 
RootAccessible -> RootDocument
AsRoot -> AsRootDocument
IsRoot -> IsRootDocument?

Comment 6

[alexander :surkov (:asurkov)]

landed on 2.0 (fx4rc) - http://hg.mozilla.org/mozilla-central/rev/82aabe4f0c3b