add downcasting for nsRootAccessible

RESOLVED FIXED

Status

()

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

People

(Reporter: surkov, Assigned: surkov)

Tracking

({access})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 515290 [details] [diff] [review]
patch
Attachment #515290 - Flags: review?(bolterbugz)
Attachment #515290 - Flags: approval2.0?
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?
(Assignee)

Comment 2

8 years ago
(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 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.
Attachment #515290 - Flags: review?(bolterbugz) → review+
(Assignee)

Comment 4

8 years ago
(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.
(Assignee)

Comment 5

8 years ago
perhaps 
RootAccessible -> RootDocument
AsRoot -> AsRootDocument
IsRoot -> IsRootDocument?
Attachment #515290 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 6

8 years ago
landed on 2.0 (fx4rc) - http://hg.mozilla.org/mozilla-central/rev/82aabe4f0c3b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.