Closed Bug 1023509 Opened 5 years ago Closed 5 years ago

only initialize platform accessibility in the main process

Categories

(Core :: Disability Access APIs, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(1 file)

No description provided.
Since platform accessibility software only interacts with the main
process there's no reason to do platform specific things in the content
processes.  Eventually we should also be able to only create generic
accessibles that don't use the platform wrapper classes, but that will
take more work so we'll keep creating them in the content processes for
now.
Attachment #8437908 - Flags: review?(dbolter)
Attachment #8437908 - Flags: review?(dbolter) → review+
Comment on attachment 8437908 [details] [diff] [review]
only initialize platform accessibility in the main process

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

::: accessible/src/base/nsAccessibilityService.cpp
@@ +1090,5 @@
>  #ifdef A11Y_LOG
>    logging::CheckEnv();
>  #endif
>  
> +  if (XRE_GetProcessType() == GeckoProcessType_Default)

it'd be nice to have IsDefaultProces() helper
(In reply to alexander :surkov from comment #2)
> Comment on attachment 8437908 [details] [diff] [review]
> only initialize platform accessibility in the main process
> 
> Review of attachment 8437908 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +1090,5 @@
> >  #ifdef A11Y_LOG
> >    logging::CheckEnv();
> >  #endif
> >  
> > +  if (XRE_GetProcessType() == GeckoProcessType_Default)
> 
> it'd be nice to have IsDefaultProces() helper

I considered that too but I sort of like it raw.
(In reply to alexander :surkov from comment #2)
> Comment on attachment 8437908 [details] [diff] [review]
> only initialize platform accessibility in the main process
> 
> Review of attachment 8437908 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +1090,5 @@
> >  #ifdef A11Y_LOG
> >    logging::CheckEnv();
> >  #endif
> >  
> > +  if (XRE_GetProcessType() == GeckoProcessType_Default)
> 
> it'd be nice to have IsDefaultProces() helper

I'd say IsMainProcess or something would make more sense, but I'd say it makes sense to add such a function to nsXULAppAPI.h.  If you want to add such a function there fine, but I don't really think its worth the time.
(In reply to Trevor Saunders (:tbsaunde) from comment #4)

> > > +  if (XRE_GetProcessType() == GeckoProcessType_Default)
> > 
> > it'd be nice to have IsDefaultProces() helper
> 
> I'd say IsMainProcess or something would make more sense,

this one is also good with me

> but I'd say it
> makes sense to add such a function to nsXULAppAPI.h.  

sure, I didn't mean to put it into a11y :)

> If you want to add
> such a function there fine, but I don't really think its worth the time.

just syntax sugar, short and nice, we may have gfb
https://hg.mozilla.org/mozilla-central/rev/27de813b3f79
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee: nobody → tbsaunde+mozbugs
You need to log in before you can comment on or make changes to this bug.