Last Comment Bug 723796 - rename nsAccessible::GetAllowsAnonChildAccessibles
: rename nsAccessible::GetAllowsAnonChildAccessibles
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: joseph
:
Mentors:
Depends on:
Blocks: cleana11y
  Show dependency treegraph
 
Reported: 2012-02-02 18:21 PST by alexander :surkov
Modified: 2012-02-08 17:59 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
GetAllowsAnonChildAccessibles renamed to CanHaveAnonChildren (5.13 KB, patch)
2012-02-06 07:13 PST, joseph
surkov.alexander: review-
Details | Diff | Review
This time, all GetAllowsAnonChildAccessibles() methods renamed to CanHaveAnonChildren() (16.61 KB, patch)
2012-02-07 14:00 PST, joseph
surkov.alexander: review+
Details | Diff | Review

Description alexander :surkov 2012-02-02 18:21:25 PST
rename nsAccessible::GetAllowsAnonChildAccessibles to something more nice.

Options:
AreAnonChildrenAllowed - short but not too correct
CanHaveChildrenInAnonContent - doesn't describe that class expects or not expect the anonymous content be accessible
AreChildrenInAnonContentAllowed
AllowsChildrenInAnonContent

Trevor, ideas?
Comment 1 Trevor Saunders (:tbsaunde) 2012-02-02 18:43:56 PST
(In reply to alexander :surkov from comment #0)
> rename nsAccessible::GetAllowsAnonChildAccessibles to something more nice.
> 
> Options:
> AreAnonChildrenAllowed - short but not too correct
> CanHaveChildrenInAnonContent - doesn't describe that class expects or not
> expect the anonymous content be accessible
> AreChildrenInAnonContentAllowed
> AllowsChildrenInAnonContent

how about CanHaveChildrenForAnonContent()?

> Trevor, ideas?

how about kill it and use a bit in mFlags for the same purpose?
Comment 2 alexander :surkov 2012-02-02 19:13:47 PST
(In reply to Trevor Saunders (:tbsaunde) from comment #1)
> (In reply to alexander :surkov from comment #0)
> > rename nsAccessible::GetAllowsAnonChildAccessibles to something more nice.
> > 
> > Options:
> > AreAnonChildrenAllowed - short but not too correct
> > CanHaveChildrenInAnonContent - doesn't describe that class expects or not
> > expect the anonymous content be accessible
> > AreChildrenInAnonContentAllowed
> > AllowsChildrenInAnonContent
> 
> how about CanHaveChildrenForAnonContent()?

mm, ok.

> > Trevor, ideas?
> 
> how about kill it and use a bit in mFlags for the same purpose?

somebody should run through implementations and make sure accessible is recreated when attribute are changed (it'd be nice to write mochitests). Anyway, if we see that accessible is not recreated then it can be a problem (in case if it's children dont' get updated).
Comment 3 alexander :surkov 2012-02-05 21:23:03 PST
after chat on irc Trevor and I are fine with CanHaveAnonChildren() name.
Comment 4 joseph 2012-02-06 07:13:45 PST
Created attachment 594702 [details] [diff] [review]
GetAllowsAnonChildAccessibles renamed to CanHaveAnonChildren
Comment 5 alexander :surkov 2012-02-07 02:01:50 PST
Comment on attachment 594702 [details] [diff] [review]
GetAllowsAnonChildAccessibles renamed to CanHaveAnonChildren

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

it appears you didn't change all entries, for example, nsXULComboboxAccessible::GetAllowsAnonChildAccessibles()

::: accessible/src/base/nsAccessible.h
@@ +387,5 @@
>     */
>    virtual nsresult HandleAccEvent(AccEvent* aAccEvent);
>  
>    /**
>     * Return true if there are accessible children in anonymous content

while you're here then please change the comment like:
Return true if this accessible allows accessible children from anonymous subtree.

::: accessible/src/mac/nsAccessibleWrap.h
@@ +98,5 @@
>    /**
>     * Returns this accessible's all children, adhering to "flat" accessibles by 
>     * not returning their children.
>     */
> +  void GetUnignoredChildren(nsTArray< nsAccessibile* > & aChildrenArray);

why is this change?
Comment 6 joseph 2012-02-07 14:00:49 PST
Created attachment 595165 [details] [diff] [review]
This time, all GetAllowsAnonChildAccessibles() methods renamed to CanHaveAnonChildren()
Comment 7 alexander :surkov 2012-02-07 17:51:02 PST
Comment on attachment 595165 [details] [diff] [review]
This time, all GetAllowsAnonChildAccessibles() methods renamed to CanHaveAnonChildren()

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

r=me
Comment 8 alexander :surkov 2012-02-07 23:28:35 PST
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/417bf64cf918
Comment 9 Ed Morley [:emorley] 2012-02-08 09:08:57 PST
https://hg.mozilla.org/mozilla-central/rev/417bf64cf918

Congratulations on your first patch in the tree! Hope to see you on IRC (http://irc.mozilla.org/) in #developers soon. If you'd like to fix another bug (we'd love it if you did!) but need some inspiration, pop on & say hi - and we'll find something for you :-)
Comment 10 alexander :surkov 2012-02-08 17:59:29 PST
(In reply to Ed Morley [:edmorley] from comment #9)
> https://hg.mozilla.org/mozilla-central/rev/417bf64cf918
> 
> Congratulations on your first patch in the tree! Hope to see you on IRC
> (http://irc.mozilla.org/) in #developers soon. If you'd like to fix another
> bug (we'd love it if you did!) but need some inspiration, pop on & say hi -
> and we'll find something for you :-)

and #accessibility channel too :)

Thanks, Joseph, for working on this!

Note You need to log in before you can comment on or make changes to this bug.