Last Comment Bug 739193 - nsTextEquivUtils::AppendTextEquivFromContent shouldn't use GetAccService()->GetAccessible
: nsTextEquivUtils::AppendTextEquivFromContent shouldn't use GetAccService()->G...
Status: RESOLVED FIXED
[good first bug][mentor=hub@mozilla.c...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Max Li [:maxli]
:
Mentors:
Depends on:
Blocks: 725572
  Show dependency treegraph
 
Reported: 2012-03-26 05:43 PDT by alexander :surkov
Modified: 2012-04-02 11:09 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (1.08 KB, patch)
2012-03-30 04:27 PDT, Max Li [:maxli]
hub: review-
Details | Diff | Review
Patch v2 (1.66 KB, patch)
2012-03-30 17:15 PDT, Max Li [:maxli]
hub: review+
tbsaunde+mozbugs: review+
Details | Diff | Review

Description alexander :surkov 2012-03-26 05:43:42 PDT
Do gInitiatorAcc->Document()->GetAccessible(aContent) instead (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsTextEquivUtils.cpp#139)

We never cross document boundaries in name calculation handled in nsTextEquivUtils so we can do that.
Comment 1 Max Li [:maxli] 2012-03-30 04:27:41 PDT
Created attachment 610849 [details] [diff] [review]
patch v1

All mochitests pass; it compiles fine. I don't know if there's any other way for me to verify I made the right change...
Comment 2 Trevor Saunders (:tbsaunde) 2012-03-30 06:15:59 PDT
you've removed the only meaningful use of shell so while your here you could stop getting the presShell all together.
Comment 3 Hubert Figuiere [:hub] 2012-03-30 09:06:54 PDT
Comment on attachment 610849 [details] [diff] [review]
patch v1

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

::: accessible/src/base/nsTextEquivUtils.cpp
@@ +135,5 @@
>    bool goThroughDOMSubtree = true;
>  
>    if (isVisible) {
>      nsAccessible* accessible =
> +      gInitiatorAcc->Document()->GetAccessible(aContent);

As Trevor said, please remove nsIPresShell line 121 since we don't use it anymore.
Comment 4 Max Li [:maxli] 2012-03-30 17:15:25 PDT
Created attachment 611095 [details] [diff] [review]
Patch v2
Comment 5 Trevor Saunders (:tbsaunde) 2012-03-30 21:32:37 PDT
Comment on attachment 611095 [details] [diff] [review]
Patch v2

looks good thanks!
Comment 6 Hubert Figuiere [:hub] 2012-03-31 15:42:48 PDT
Comment on attachment 611095 [details] [diff] [review]
Patch v2

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

looks good
Comment 8 Matt Brubeck (:mbrubeck) 2012-04-02 11:09:20 PDT
https://hg.mozilla.org/mozilla-central/rev/8869f7a21a63

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