Last Comment Bug 739193 - nsTextEquivUtils::AppendTextEquivFromContent shouldn't use GetAccService()->GetAccessible
: nsTextEquivUtils::AppendTextEquivFromContent shouldn't use GetAccService()->G...
[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]
: alexander :surkov
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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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

Description User image alexander :surkov 2012-03-26 05:43:42 PDT
Do gInitiatorAcc->Document()->GetAccessible(aContent) instead (

We never cross document boundaries in name calculation handled in nsTextEquivUtils so we can do that.
Comment 1 User image 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 User image 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 User image 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 User image Max Li [:maxli] 2012-03-30 17:15:25 PDT
Created attachment 611095 [details] [diff] [review]
Patch v2
Comment 5 User image Trevor Saunders (:tbsaunde) 2012-03-30 21:32:37 PDT
Comment on attachment 611095 [details] [diff] [review]
Patch v2

looks good thanks!
Comment 6 User image 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 User image Matt Brubeck (:mbrubeck) 2012-04-02 11:09:20 PDT

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