Last Comment Bug 785510 - [AccessFu] only instantiate a11y engine on AccessFu._enable()
: [AccessFu] only instantiate a11y engine on AccessFu._enable()
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Eitan Isaacson [:eeejay]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-24 14:15 PDT by Eitan Isaacson [:eeejay]
Modified: 2012-08-27 01:59 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add accessible retreival service to utils, and remove it as global from modules. (9.89 KB, patch)
2012-08-24 14:22 PDT, Eitan Isaacson [:eeejay]
dbolter: review+
Details | Diff | Splinter Review

Description Eitan Isaacson [:eeejay] 2012-08-24 14:15:27 PDT
Some globals have us instantiating a11y early (understatement).
Comment 1 Eitan Isaacson [:eeejay] 2012-08-24 14:22:32 PDT
Created attachment 655162 [details] [diff] [review]
Add accessible retreival service to utils, and remove it as global from modules.

Didn't test this much. But I think this is good, at least before uplift.
Comment 2 David Bolter [:davidb] 2012-08-24 19:30:05 PDT
Comment on attachment 655162 [details] [diff] [review]
Add accessible retreival service to utils, and remove it as global from modules.

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

I didn't apply and test this patch as I'm assuming you did. I assume grepping for global gAccRetrieval declarations (you deleted the 3 I found), and usage now results in the empty set.

I'm assuming there is no sign of engine instantiation anymore.

Thanks for jumping on this (yoikes).

r=me

::: accessible/src/jsat/Utils.jsm
@@ +22,5 @@
>  
> +  get AccRetrieval() {
> +    if (!this._AccRetrieval)
> +      this._AccRetrieval = Cc['@mozilla.org/accessibleRetrieval;1'].
> +          getService(Ci.nsIAccessibleRetrieval);

nit: Since this spans two lines I'd like to see {}'s or a space before the return.
Comment 3 Eitan Isaacson [:eeejay] 2012-08-26 20:15:35 PDT
Addressed nits

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6192cb67e8c
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-08-27 01:59:23 PDT
https://hg.mozilla.org/mozilla-central/rev/d6192cb67e8c

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