Closed Bug 785047 Opened 12 years ago Closed 12 years ago

OOPP for Flash breaks some blind and low vision assistive technologies

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Version:
14 Branch
Platform:
x86
Windows Vista
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18

People

(Reporter: richschwer, Assigned: surkov)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

patch2
3.85 KB, patch
jaas
: review+
Details | Diff | Splinter Review
build bustage fix, v1
988 bytes, patch
jaas
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

Firefox introduced a regression in version 14 in bug 769721 that prohibits an assistive technology, such as JAWS or NVDA from being able to provide access to Flash within the same process. 


Actual results:

The modification locks down the Flash player into running in a separate process by removing the vehicle to disable this feature. Prior to this regression a screen reader could force Firefox to keeping Flash in the same process. Having built screen readers I can tell you that this creates real problems for screen reader users.

1. When the screen reader hooks into the browser it hooks in process to be able to be performant. When Flash runs in a separate process it means that the screen reader needs to access the Flash process out of process make the user experience incredibly slow as it introduces a context switch. Context switching is subject to OS scheduling and memory mapping overhead that persists regardless of how fast your processor runs. This change negatively impacted the performance of both JAWS and NVDA.
2. By sticking Flash in a separate process you make it a separate model entry. Screen Readers do not merge virtual buffers and interactions across processes. Think trying to merge an MSWord document view with Firefox. It is a usability disaster as you are mixing two different applications. Yet, unlike Word, Flash should be integrated with seamlessly with the web page yet now you are forcing the screen reader to separate two parts of the Web experience - a usability disaster. I am sure NVDA is also not happy about this.

While I understand the perceived security problem I strongly urge you to ask Mozilla to put feature back into 14 that allows the screen reader to have Firefox put Flash in the same process.
Blocks: 769721
Status: UNCONFIRMED → NEW
Component: Untriaged → Disability Access APIs
Ever confirmed: true
Keywords: regression
OS: Mac OS X → Windows Vista
Product: Firefox → Core
NVDA hasn't integrated the Flash content into the virtual buffer for quite a while, at least late 2009, IIRC. With NVDA, you put the virtual cursor on the "embedded object" that is the Flash thing, then press Enter to actually go into that Flash. Likewise, NVDA+Ctrl+pace will take you out and back to the parent document.

Having Flash out of process has not caused problems for NVDA. The one screen reader I know that needed this workaround to disable OOP for Flash was JAWS.
NVDA never merged Flash into the main buffer. We consider it to be separate (but embedded) content that you can easily enter and exit. Thus, we still access it in-process, as we are injected into the Flash process and render a buffer there.
let's do whitelisting for consumers we are aware about
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #654946 - Flags: review?(joshmoz)
Summary: Firefox introduced a regression introduced in 769721 that breaks blind and low vision assistive technologies → OOPP for Flash breaks some blind and low vision assistive technologies
Comment on attachment 654946 [details] [diff] [review]
force disable oop when JAWS is running

AIUI, Flash 11.3 and later in protected mode already don't work with a11y because of the sandbox. What set of users are we actually trying to help here?
@Benjamin The problem here is out of process access by ATs impacting performance. screen readers need to run in process to get the performance needed. Firefox forced it into a sandbox (separate process) with a regression. JAWS hooks into FF process and once there can put all in the same model. Flash apps often interact with web apps so it is important to run in context (process). That is not the case with say an office plug-in.
Flash moved itself into *another* process. So removing plugin-container from the equation isn't going to help you at all, if you have a modern Flash. And Adobe isn't testing the in-process configuration any more and has basically broken it in a number of ways, including some interesting deadlocks.
lovely.
Comment on attachment 654946 [details] [diff] [review]
force disable oop when JAWS is running

canceling review until we figure out the next step
Attachment #654946 - Flags: review?(joshmoz)
In perspective of blind, hi-end user (but not a developer). Mozilla developers forced me to downgrade Firefox to 13.1 with latest version of flash. Protected mode in Adobe Flash (sandbox) can be disabled as I done in mms.cfg by putting ProtectedMode=0 line. In this configuration (ff 13 + flash 11.4) everything works well. I think Mozilla have to provide alternative method for disabling plugin-container.exe process too. Waiting for fixing the problem then installing latest Firefox.
Attachment #654946 - Flags: review?(benjamin)
Marco, can you do a testing please?
Alexander, thanks for the build, now testing.
Ok, everything works correctly. Flash version: 11.4.402.265 with ProtectedMode set to 0, Freedom Scientific Jaws version: 13.0.1005
Benjamin, ping?
I do not think that this is the correct decision: if the user has not configured *both* Flash protected mode off and OOPP off, it is easy to completely destabilize Firefox, and neither of those configuration options are exposed.

Josh is the module owner, perhaps he can work out an acceptable compromise, but this solution is basically asking for untested behavior and instability for a subset of users.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #17)
> I do not think that this is the correct decision: if the user has not
> configured *both* Flash protected mode off and OOPP off, it is easy to
> completely destabilize Firefox, and neither of those configuration options
> are exposed.
> 
> Josh is the module owner, perhaps he can work out an acceptable compromise,
> but this solution is basically asking for untested behavior and instability
> for a subset of users.

If they didn't configured Flash or if we don't take solution like proposed one then this subset of users gets a slow and crashy browser.

I'm fine with either solution that fixes a bug but please it shouldn't take long. Users downgrades to Firefox 13 (see comment #13), that's not what we want.
Comment on attachment 654946 [details] [diff] [review]
force disable oop when JAWS is running

redirecting review request to Josh per Benjamin's comment #17.
Attachment #654946 - Flags: review?(benjamin) → review?(joshmoz)
The problem with the current patch, as I understand Benjamin's point, is that we can't guarantee that jaws users also configured Flash properly (disabling OOP). If they didn't do that, Firefox will be unstable in addition to jaws not working. Is this accurate?

The current patch limits the behavior change to jaws users, which I imagine is a very small number of people relative to our entire user base. I don't have a good sense of how many actual people we're talking about though, relative count aside. 100? 1000? 50,000? 250,000?

Am I correct in my understanding that nvda and jaws are more or less equivalent alternative assistive technologies, and that our changes only negatively impacted jaws?
I can clearly say I know at least twenty of my friends who has this problem too. Comparing this, I am sure there are hundreds of thouzents people having this problem, but nearly none of them knows about Bugzilla. Moreover, few of my friends stopped using Firefox and returned to IE because they can't understand this. Freedom Scientific (company which owns Jaws) published resolution which works on older versions of Firefox(13.0)  and Flash (11.3) which can be found at here: http://www.freedomscientific.com/fs_support/BulletinView.asp?QC=1410
We can assume that Freedom Scientific has large number of requests about OOP, so please don't disallow us from using your browser. I can't contact FS for number of reported Flash problems. I can also say that Jaws is the most popular screen reader in the world and most configurable because of advanced scripting possibilities.

I have the question:
Is it possible to determine by Firefox if Flash is running in ProtectedMode?
If yes, OOP for flash would be enabled depending on Protected Mode.
If there is no way to detect it, special booleon (under different name) in about:config could be added to handle this issue. So, any unexperienced user couldn't switch it by falt.
Sorry in advance if I'm not right.
(In reply to Josh Aas (Mozilla Corporation) from comment #20)
> The problem with the current patch, as I understand Benjamin's point, is
> that we can't guarantee that jaws users also configured Flash properly
> (disabling OOP). If they didn't do that, Firefox will be unstable in
> addition to jaws not working. Is this accurate?

Yes. Would it be ok if we respect the preference again for JAWS users only? 

> The current patch limits the behavior change to jaws users, which I imagine
> is a very small number of people relative to our entire user base. I don't
> have a good sense of how many actual people we're talking about though,
> relative count aside. 100? 1000? 50,000? 250,000?

Unfortunately I don't know. Afaik telemetry doesn't show us numbers yet. But JAWS is still a leader on the market (49% of markershare in May 2012 per http://webaim.org/projects/screenreadersurvey4/)

> Am I correct in my understanding that nvda and jaws are more or less
> equivalent alternative assistive technologies, and that our changes only
> negatively impacted jaws?

yes
The best fix is to clean the web of all flash.

The second best fix is for Freedom Scientific to fix the issue on their side.

I think we are discussing the alternatives, which is unfortunate but we don't like the thought of Jaws users having to downgrade/move.

From our side we can do:

1. Nothing, leaving pressure for one of the better fixes.
2. Disable all flash for Jaws users. They will have the most secure browsing environment ever.
3. Circumvent Adobe and Mozilla security measures for Jaws users (the current patch). Untested code paths are worrisome. At least Jaws users might still use (possibly critical) apps, and if they are behind a firewall they might be secure.

Are there others?
Comment on attachment 654946 [details] [diff] [review]
force disable oop when JAWS is running

I'm concerned about putting Flash in-process for jaws users and destabilizing Firefox when we can't do more to ensure that the tradeoff of stability for benefits is complete - that putting Flash in-process will actually help because Flash has been configured without sandboxing as well.

I'd be more for doing something on our end if *no* assistive software could work under the current config, but it seems that is not the case.

Benjamin's patch that forces Flash OOP for Vista+ users is not configurable, it entirely overrides all prefs. Another version of that fix, not as airtight, would have been to simply change the pref name to reset it. If we simply had a new pref then jaws users could change it at the same time they change the Flash pref, and by requiring that we'd have more confidence that jaws users were knowingly buying into the tradeoff. If we're going to pursue this further on our end this is probably the angle to consider - introducing a pref with a new name that could be flipped in about:config. I'm still not sure that'd be the right thing to do, but I'm pretty sure I don't want to automatically put Flash in-process for anyone running jaws.
Attachment #654946 - Flags: review?(joshmoz) → review-
I'd agree it would have to be opt-in somehow.
Josh, getting back to the question from comment #22: will the old pref respect for JAWS users work for you? what means nothing is changed from JAWS users perspective. 

We could change prefs for this and it's a solution that works but I'm not sure what benefits we get. Users have to investigate how to make their browser work.
(In reply to alexander :surkov from comment #26)

> We could change prefs for this and it's a solution that works but I'm not
> sure what benefits we get. Users have to investigate how to make their
> browser work.

Fixing this on our end doesn't make anything work for users on its own, they still have to mess with Flash. If you can do that you can definitely flip a pref in Firefox, which in turn signals buy-in to us. The explicit buy-in is the benefit.
So new pref + for JAWS only is the way to go?
Josh, ping? (previous comment).
(In reply to alexander :surkov from comment #29)
> Josh, ping? (previous comment).

If we were going to do something on our end then a pref would be it.

I don't want to add to the list of reasons someone might run plugin in-process though, so I still think JAWS should just fix their software. Seems like the other assistive software works fine, so it's possible for JAWS to fix this. I feel like if we create a workaround on our end then JAWS will just ignore this and JAWS users will be stuck with in-process Flash for a long time. It's just not OK to run plugins in-process on the web today.
Solving internal bugs, problems and security issues by external processes is not wight way of professional programming.
(In reply to Josh Aas (Mozilla Corporation) from comment #30)
> (In reply to alexander :surkov from comment #29)
> > Josh, ping? (previous comment).
> 
> If we were going to do something on our end then a pref would be it.
> 
> I don't want to add to the list of reasons someone might run plugin
> in-process though, so I still think JAWS should just fix their software.
> Seems like the other assistive software works fine, so it's possible for
> JAWS to fix this. I feel like if we create a workaround on our end then JAWS
> will just ignore this and JAWS users will be stuck with in-process Flash for
> a long time. It's just not OK to run plugins in-process on the web today.

If I understand right then the issue isn't really easy and plain and we/they need some time to come with final decision/solution. We stopped them working so suddenly and thus I think we should get back a pref for some period (I'm perfectly fine if we start planning to drop that pref at all for certain Firefox version).
Attached patch patch2 — — Splinter Review 

        Attachment #654946 -
        Attachment is obsolete: true

        Attachment #661747 -
        Flags: review?(joshmoz)

    
    

    
  
(In reply to alexander :surkov from comment #32)

> If I understand right then the issue isn't really easy and plain and we/they
> need some time to come with final decision/solution. We stopped them working
> so suddenly and thus I think we should get back a pref for some period (I'm
> perfectly fine if we start planning to drop that pref at all for certain
> Firefox version).

I'd be open to coming up with an (aggressive) date on which the pref gets removed, communicating that, and sticking to it. We can take something like the current patch on m-c and up to beta if we can get approvals.

I propose that we back it out at the very beginning of the Firefox 19 release cycle. That gives JAWS around 20+ weeks to come up with a fix, and they've already had around 9 weeks to get started. This seems plenty long enough if they really prioritize resolving the issue. This already worries me as it means we're severely destabilizing our product for lots of people for 18 weeks, so I don't think I'd agree to any more time than that.

Does this help enough with the problem? Is it worth doing?

    
    

    
  
This proposal sounds reasonable to me.

    
    

    
  
Comment on attachment 661747 [details] [diff] [review]
patch2

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

r+, but before we land this I want us to agree on a date for the backout and I want to see that it has been communicated to the JAWS developers. Would be great if a JAWS developer could comment here to acknowledge.

Thanks for working on this, not an ideal solution but I'm glad we could work something out.

        Attachment #661747 -
        Flags: review?(joshmoz) → review+

    
    

    
  
(In reply to Josh Aas (Mozilla Corporation) from comment #36)

> r+, but before we land this I want us to agree on a date for the backout and
> I want to see that it has been communicated to the JAWS developers. Would be
> great if a JAWS developer could comment here to acknowledge.

(sorry for the late reply, it got lost in my queue)

There are bunch of things affecting on decision, for example, JAWS is pretty expensive software, it used by enterprise a lot, the technical solution how to workaround the issue is not evident. I think it makes sense if you join the discussion with FS to figure out a date. In the meantime I suggest to land this patch as it is. Does it work for you?

    
    

    
  
in other words: users get an option to make their software working while we figure out the solution.

    
    

    
  
(In reply to alexander :surkov from comment #38)
> in other words: users get an option to make their software working while we
> figure out the solution.

I don't want to land the fix without a plan and a backout date up front, I want to know exactly what we're getting into first.

    
    

    
  
(In reply to Josh Aas (Mozilla Corporation) from comment #39)
> (In reply to alexander :surkov from comment #38)
> > in other words: users get an option to make their software working while we
> > figure out the solution.
> 
> I don't want to land the fix without a plan and a backout date up front, I
> want to know exactly what we're getting into first.

Figuring it out may take days or maybe weeks. We can trade I think. Let's land the patch now and backout it in one month if we don't have a plan/agreement on stopping this pref work. Sounds ok?

    
    

    
  
OK - land it now but it comes out of beta and aurora in one month if we don't have a confirmed plan with the Jaws developers, and I won't agree to a plan that lets it stay in longer than Firefox 18. So it will get backed out of mozilla-central in ~15 days either way. That gives the Jaws developers 20+ weeks to fix this.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/7f64b635c8d6
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED

    
    

    
  


  

        Attachment #667751 -
        Flags: review?(joshmoz)

    
    

    
  
Comment on attachment 667751 [details] [diff] [review]
build bustage fix, v1

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

thank you for fixing it

::: dom/plugins/base/nsNPAPIPlugin.cpp
@@ +273,5 @@
>    // On Windows Vista+, we force Flash to run in OOPP mode because Adobe
>    // doesn't test Flash in-process and there are known stability bugs.
>    if (aPluginTag->mIsFlashPlugin && IsVistaOrLater()) {
>  #ifdef ACCESSIBILITY
> +    useA11yPref =  a11y::Compatibility::IsJAWS();

you don't need this line

    
    

    
  
Comment on attachment 667751 [details] [diff] [review]
build bustage fix, v1

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

r+ with the unnecessary line removed

::: dom/plugins/base/nsNPAPIPlugin.cpp
@@ +273,5 @@
>    // On Windows Vista+, we force Flash to run in OOPP mode because Adobe
>    // doesn't test Flash in-process and there are known stability bugs.
>    if (aPluginTag->mIsFlashPlugin && IsVistaOrLater()) {
>  #ifdef ACCESSIBILITY
> +    useA11yPref =  a11y::Compatibility::IsJAWS();

agreed

        Attachment #667751 -
        Flags: review?(joshmoz) → review+

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: