Last Comment Bug 726005 - IDRefsIterator::Next() should use nsDocAccessible::GetAccessible()
: IDRefsIterator::Next() should use nsDocAccessible::GetAccessible()
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jignesh Kakadiya [:jhk]
:
Mentors:
Depends on:
Blocks: 725572
  Show dependency treegraph
 
Reported: 2012-02-10 08:02 PST by alexander :surkov
Modified: 2012-03-23 06:00 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (10.62 KB, patch)
2012-03-17 10:17 PDT, Jignesh Kakadiya [:jhk]
tbsaunde+mozbugs: review+
surkov.alexander: feedback+
Details | Diff | Splinter Review
Patch (10.70 KB, patch)
2012-03-20 22:14 PDT, Jignesh Kakadiya [:jhk]
no flags Details | Diff | Splinter Review
patch (10.70 KB, patch)
2012-03-22 20:02 PDT, Jignesh Kakadiya [:jhk]
no flags Details | Diff | Splinter Review

Description alexander :surkov 2012-02-10 08:02:43 PST
For that IDRefsIterator constructor (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/AccIterator.h#264) should take nsDocAccessible instance.

Trevor, sounds good?
Comment 1 Trevor Saunders (:tbsaunde) 2012-02-10 09:12:17 PST
seems fine.
Comment 2 Jignesh Kakadiya [:jhk] 2012-03-16 10:23:13 PDT
http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsTextEquivUtils.cpp#97
Is there any way to pass nsDocAccessible* here?
Comment 3 Jignesh Kakadiya [:jhk] 2012-03-17 10:17:47 PDT
Created attachment 606883 [details] [diff] [review]
patch

Added nsDocAccessible* as argument in IDRefsIterator constructor.
Comment 4 alexander :surkov 2012-03-20 00:35:16 PDT
(In reply to Jignesh Kakadiya [:jhk] from comment #2)
> http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/
> nsTextEquivUtils.cpp#97
> Is there any way to pass nsDocAccessible* here?

yeah, get it from aAccessible (aAccessible->Document())
Comment 5 alexander :surkov 2012-03-20 00:43:48 PDT
Comment on attachment 606883 [details] [diff] [review]
patch

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

sounds good, thanks!

::: accessible/src/base/AccIterator.h
@@ +293,5 @@
>  
>    nsString mIDs;
>    nsIContent* mContent;
>    nsAString::index_type mCurrIdx;
> +  nsDocAccessible* mDoc;

I think Trevor will say something about memory aligning

::: accessible/src/base/nsAccessible.cpp
@@ +2042,1 @@
>                                          nsGkAtoms::aria_describedby));

fix indentation while you're here
Comment 6 Trevor Saunders (:tbsaunde) 2012-03-20 19:13:06 PDT
Comment on attachment 606883 [details] [diff] [review]
patch

r=me with surkov's comments addressed
Comment 7 Jignesh Kakadiya [:jhk] 2012-03-20 22:14:11 PDT
Created attachment 607859 [details] [diff] [review]
Patch

Fixed indentation.
Comment 8 alexander :surkov 2012-03-20 22:47:05 PDT
(In reply to alexander :surkov from comment #5)
> ::: accessible/src/base/AccIterator.h
> @@ +293,5 @@
> >  
> >    nsString mIDs;
> >    nsIContent* mContent;
> >    nsAString::index_type mCurrIdx;
> > +  nsDocAccessible* mDoc;
> 
> I think Trevor will say something about memory aligning

what about this?
Comment 9 Jignesh Kakadiya [:jhk] 2012-03-22 09:43:13 PDT
(In reply to alexander :surkov from comment #8)
> (In reply to alexander :surkov from comment #5)
> > ::: accessible/src/base/AccIterator.h
> > @@ +293,5 @@
> > >  
> > >    nsString mIDs;
> > >    nsIContent* mContent;
> > >    nsAString::index_type mCurrIdx;
> > > +  nsDocAccessible* mDoc;
> > 
> > I think Trevor will say something about memory aligning
> 
> what about this?
I think alignment is proper similar in other files(nsAccessNode.h). Let me know if it requires more changes. thanks:)
Comment 10 alexander :surkov 2012-03-22 16:53:54 PDT
(In reply to Jignesh Kakadiya [:jhk] from comment #9)
> > what about this?
> I think alignment is proper similar in other files(nsAccessNode.h). Let me
> know if it requires more changes. thanks:)

see bug 734566 description for example
Comment 11 Jignesh Kakadiya [:jhk] 2012-03-22 20:02:46 PDT
Created attachment 608583 [details] [diff] [review]
patch

fixed Alignment.
Comment 13 Marco Bonardo [::mak] (Away 6-20 Aug) 2012-03-23 06:00:33 PDT
https://hg.mozilla.org/mozilla-central/rev/de43aa36ceef

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