Closed Bug 727940 Opened 8 years ago Closed 8 years ago

Windows app can't read Firefox setting for hardware acceleration (D2D)

Categories

(Core :: Disability Access APIs, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: jeckhardt, Assigned: surkov)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120208012847

Steps to reproduce:

I'm trying to determine from a Windows app if the hardware acceleration is turned on or off in Firefox


Actual results:

I can't find documentation about an available interface


Expected results:

I need an interface to read the "hardware acceleration" setting
Filing this in accessibility for initial triage.
Component: Untriaged → Disability Access APIs
Product: Firefox → Core
QA Contact: untriaged → accessibility-apis
Version: 11 Branch → unspecified
Status: UNCONFIRMED → NEW
Ever confirmed: true
All, note there is a companion bug 727942.

Jost, is there any precedent for this kind querying with other windows apps?
Sometimes these settings are stored in the registry. 
The IOmNavigator interface in the IE DOM provides this access.

So maybe I need Firefox DOM access?
(In reply to Jost Eckhardt, Ai Squared from comment #3)
> Sometimes these settings are stored in the registry. 
> The IOmNavigator interface in the IE DOM provides this access.

What object implement this interface? If Firefox doesn't have existing option for this then it makes sense to implement it. Note, that 'if' should be checked and this is out of accessibility triage (David, cc someone).

Alternatively IOmNavigator looks like upperset of IA2's IAccessibleApplication interface. So we could request IA2 group to extend it by introducing new methods. Or we can make sure IAccessibleApplication object implements IAccessible2 (not everybody likes this but that's a current Firefox implementation) and then use object attributes to get any setting you need.

Btw, Jost, thank you for bugs filling (sometimes it works better than emails, sorry for that).
I like the object attributes idea. Very flexible.
note: on #developers they said: is accessing gfx.direct2d.disabled and gfx.font_rendering.directwrite.enabled sufficient? So we can expose them via a11y API.
So the proposal is:
expose "D2D" object attribute (IAccessible2) on application accessible object (implementing IAccessibleApplication interface). Possible values: true and false.

Are there any objections?
Attached patch patch (obsolete) — Splinter Review
so if nobody has objections
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #598242 - Flags: review?(trev.saunders)
Attachment #598242 - Flags: review?(marco.zehe)
Comment on attachment 598242 [details] [diff] [review]
patch

r=me
Attachment #598242 - Flags: review?(marco.zehe) → review+
My biggest concern with this approach is that it requires our accessibility engine to be invoked. It might be better to have an alternate way of getting this info, or we could whitelist who gets it (based on dll injection detection). Does ZoomText inject a dll?

Regarding the mochitest, how can we be sure this will always be false on our test hardware?
(In reply to alexander :surkov from comment #6)
> note: on #developers they said: is accessing gfx.direct2d.disabled and
> gfx.font_rendering.directwrite.enabled sufficient? So we can expose them via
> a11y API.

yes
(In reply to David Bolter [:davidb] from comment #10)
> My biggest concern with this approach is that it requires our accessibility
> engine to be invoked. It might be better to have an alternate way of getting
> this info, or we could whitelist who gets it (based on dll injection
> detection). Does ZoomText inject a dll?
> 


yes we do inject a DLL, not currently signed but it would not problem to sign it.
(In reply to David Bolter [:davidb] from comment #10)
> My biggest concern with this approach is that it requires our accessibility
> engine to be invoked. It might be better to have an alternate way of getting
> this info, or we could whitelist who gets it (based on dll injection
> detection). Does ZoomText inject a dll?

I started from assumption ZoomText uses IAccessible2/MSAA already. If that's not true then probably we should come with other solution.

> Regarding the mochitest, how can we be sure this will always be false on our
> test hardware?

we can't be sure but the same time we don't test 'false' there.
(In reply to Jost Eckhardt, Ai Squared from comment #12)
> yes we do inject a DLL, not currently signed but it would not problem to
> sign it.

btw, we need to expose it through Telemetry, what's a DLL name?
(In reply to alexander :surkov from comment #13)
> (In reply to David Bolter [:davidb] from comment #10)
> > My biggest concern with this approach is that it requires our accessibility
> > engine to be invoked. It might be better to have an alternate way of getting
> > this info, or we could whitelist who gets it (based on dll injection
> > detection). Does ZoomText inject a dll?
> 
> I started from assumption ZoomText uses IAccessible2/MSAA already. If that's
> not true then probably we should come with other solution.

Sorry I wasn't explicit. My concern is that we might expose information non-AT will be interested in and cause a performance hit for more users than necessary, similar to how anti-spyware apps like to know when a password field has focus (by using MSAA).
(In reply to David Bolter [:davidb] from comment #15)

> Sorry I wasn't explicit. My concern is that we might expose information
> non-AT will be interested in and cause a performance hit for more users than
> necessary, similar to how anti-spyware apps like to know when a password
> field has focus (by using MSAA).

that's always a concern, a11y APIs looks so appeal for non-a11y software developers. I think all we have is evangelism, I don't think Gecko is going to expose some COM interface for stuffs like this.
(In reply to alexander :surkov from comment #14)
> (In reply to Jost Eckhardt, Ai Squared from comment #12)
> > yes we do inject a DLL, not currently signed but it would not problem to
> > sign it.
> 
> btw, we need to expose it through Telemetry, what's a DLL name?

I email you
CC+ Sid for privacy audit.
(In reply to David Bolter [:davidb] from comment #18)
> CC+ Sid for privacy audit.

fwiw I don't think there is really any privacy concern here since if you can call get_Attributes() your running code at the same privladge level as Firefox so you could say snoop every key sent to  firefox, or any number of other evil things much worse than finding out if firefox is using hardware accel grafix.

(In reply to alexander :surkov from comment #16)
> (In reply to David Bolter [:davidb] from comment #15)
> 
> > Sorry I wasn't explicit. My concern is that we might expose information
> > non-AT will be interested in and cause a performance hit for more users than
> > necessary, similar to how anti-spyware apps like to know when a password
> > field has focus (by using MSAA).
> 
> that's always a concern, a11y APIs looks so appeal for non-a11y software
> developers. I think all we have is evangelism, I don't think Gecko is going
> to expose some COM interface for stuffs like this.

yeah, it might make more sense in a perfect world for this to be part of some sort of stable api, but we don't have one of those and I don't think this is the time or place to deal with that.
Comment on attachment 598242 [details] [diff] [review]
patch

My main concern here is what we do on !windows, otherwise I'm fine with this.  For example adding this attribute on linux would be strange and possibly confusing.

I could live with an ifdef, but I think putting this in msaa/ is the right way to do it.  If you override GetAttributes() on nsApplicationAccessible I believe you could even test it.  I wouldn't be thrilled by it, but I could live with not testing this until we have platform api tests since all of the parts are well tested already and the integration is trivial.  What you really want to test that we expose it to msaa / ia2  correctly requires a platform test already.

I'm not sure how we feel about latform specific attributes at this point, but I know we have comments lying around saying we should do things in such a way that xpcom interfaces do the same thing on all platforms.
Attachment #598242 - Flags: review?(trev.saunders)
(In reply to Trevor Saunders (:tbsaunde) from comment #19)
> (In reply to David Bolter [:davidb] from comment #18)
> > CC+ Sid for privacy audit.
> 
> fwiw I don't think there is really any privacy concern here since if you can
> call get_Attributes() your running code at the same privladge level as
> Firefox so you could say snoop every key sent to  firefox, or any number of
> other evil things much worse than finding out if firefox is using hardware
> accel grafix.

This sounds quite reasonable for me. Anyway the point was raised and we need to get feedback from privacy team.

Sid, please let me know if you need any information to answer the call.
Attached patch patch2Splinter Review
Attachment #598242 - Attachment is obsolete: true
Attachment #598887 - Flags: review?(trev.saunders)
Adding the privacy-review-needed keyword to get this in the privacy queue.  After reading the bug, I'm not sure I fully understand.  To *what* we are providing this data? Web content, add-ons, other Win.exes running on the same system, or ...?
(In reply to Sid Stamm [:geekboy] from comment #23)
> Adding the privacy-review-needed keyword to get this in the privacy queue. 
> After reading the bug, I'm not sure I fully understand.  To *what* we are
> providing this data? Web content, add-ons, other Win.exes running on the
> same system, or ...?

add-ons and other Win.exes running on the same system
Attachment #598887 - Flags: review?(trev.saunders) → review+
I don't think there's much privacy impact here.
https://hg.mozilla.org/mozilla-central/rev/d20e343d885d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.