340841 - Infinite loop with some plugins when screen reader is running -- empty tabs keep opening

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Kranz]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 2.0.50727)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

The default Download Action for Acrobat Reader is to use the plug-in. If you load a .pdf file into Firefox the content shows inside the browser as expected. But if you are running a screen reader such as JAWS or Microsoft Narrator when the .pdf file is loaded, a separate Acrobat Reader window will be opened instead. This was discovered when using the Curl RTE plug-in (http://www.curl.com/products/download.php) with .curl files because there is no external viewer fo .curl files. The content must be displayed inside of the browser. Attempting to launch the Curl RTE as an external viewer results in the Curl RTE asking the browser to show the content, which results in an infinite tab loop in Firefox. This behavior does not happen with Internet Explorer.

Reproducible: Always

Steps to Reproduce:
1. Run Narrator
2. Load a .pdf file into Firefox
3.

Actual Results:  
.pdf file is loaded into a separate window
.curl file causes infinite tab loop in Firefox

Expected Results:  
.pdf file is shown inside the browser as it is when Narrator is not running
.curl file is shown inside the browser as it is when Narrator is not running

I am not sure how to characterize the severity. It basically causes the browser to crash when browsing a .curl file while using screen readers but does not happen otherwise. Note that the windows created by the Curl RTE (in the currently released version) do not handle WM_GETOBJECT.

Comment 1

[Aaron Leventhal]

Okay, now I remember the cause. The code was checked in for bug 303841.
Here are the offending lines:
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebNavigationInfo.cpp#151

Basically when a screen reader is running and an external handler exists for a full page plugin, we try to automatically open up the content in the external handler.  The external handlers tend to be more accessible than running straight in the browser.

Possible fixes:
1) Only do this for PDF & related file types
2) Find a better external handler check. Perhaps the external handler in this case is Mozilla/Firefox itself, which should clue us in.

Comment 2

[Aaron Leventhal]

> Expected Results:  
> .pdf file is shown inside the browser as it is when Narrator is not running
> .curl file is shown inside the browser as it is when Narrator is not running

So this is not quite correct. The PDF file should continue to open in a separate viewer when a screen reader is running.

Comment 3

[Aaron Leventhal]

Biesi, what do I need to do so that the check for an external application to handle the full-page plugin type exists?

Right now I'm using:
mimeInfo->GetHasDefaultHandler(&hasDefaultHandler);

But when the file gets thrown out to the OS it comes back and tries to load content in a new firefox tab, which repeats the same check and causes an endless loop of new tabs opening.

Comment 4

[Aaron Leventhal]

Attachment: Remove special plugin handling when screen reader present

Unless we can come up with an accurate check for an external (not the current app!) handler, we need to remove this.

In reality this code does not help us with our keynav problems. When a screen reader is present, it handles the keynav into the plugin anyway. This solution doesn't help us with Flash, and I don't think it helps us with PDF, but we should check that.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

sorry, I didn't see comment 3 before. There's not really a way to check that the helper app is not the current browser... having such a way would allow to fix a few other bugs as well.

Comment 6

[David Kranz]

It should be noted that in the case of the Curl mime type, the external application is not actually the browser, but is a helper app that invokes the default browser. I'm not sure how you could find out whether the registered plugin .exe will try to recursively invoke the browser or not except by having an interface by which the plugin can be asked. It seems that you really want to ask the plugin: for accessibility purposes, do you prefer to use an external viewer even though the user configured Firefox to show the contents inline?, with the default being "no". Thanks for fixing this.

Comment 7

[Aaron Leventhal]

David, thanks for the info.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 228773 [details] [diff] [review]
Remove special plugin handling when screen reader present

sr=jst

Comment 9

[Aaron Leventhal]

Attachment: Don't give up on preferring external plugins, just don't follow that path for external requests (needs testing)

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

doesn't an EXTERNAL load type get changed to NORMAL pretty early?

Comment 11

[Aaron Leventhal]

I don't know. I haven't set the review flags for it yet because I plan to test it still.

Comment 12

[Aaron Leventhal]

(In reply to comment #10)
> doesn't an EXTERNAL load type get changed to NORMAL pretty early?
Yes this is annoying!

Here's the code that does it:
        // reset loadType so we don't have to add lots of tests for
        // LOAD_NORMAL_EXTERNAL after this point
        aLoadType = LOAD_NORMAL;
    }

The comments say it's just a convenience, but this is removing information I need. In fact I've seen this infinite loop without a screen reader running, when certain files were deleted out of the plugins directory. IMO we should be able to avoid sending a request out to the OS when that's where it's coming from.

Comment 13

[Aaron Leventhal]

It turns out that PDF test file I was using was the problem. Checking this in fixes the bug and PDF's are still accessible to screen readers. This is a good fix.

Comment 14

[Aaron Leventhal]

Comment on attachment 229693 [details] [diff] [review]
Don't give up on preferring external plugins, just don't follow that path for external requests (needs testing)

In the end we used the simple fix of backing stuff out, as PDF's are now accessible directly inside the browser. All of the talk about LOAD_NORMAL_EXTERNAL was for a different idea for a fix which had too many problems.

Comment 15

[Tracy Walker [:tracy]]

reporter or Aaron, please verify with 2.0b2 builds.  thanks