Avoid the wmode windowed -> opaque Flash workaround when we detect Flash async drawing support

RESOLVED FIXED in Firefox 49

Status

()

Core
Plug-ins
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48+ wontfix, firefox49+ fixed, firefox50+ fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
In newer versions of Flash that have support, Flash will convert windowed mode plugins to a new windowless async rendering model. This switch will be opaque to Firefox.

Firefox currently forces windowless in a few cases to avoid windowed plugin issues which this new mode addresses, so we can stop doing this for this new version of Flash.

Here's the workaround code in Firefox - 

http://searchfox.org/mozilla-central/rev/970569ad57ac4436ff31aa2ac63f38ed3ee2932d/dom/plugins/ipc/PluginModuleParent.cpp#2680-2700

To detect the new Flash mode, we'll need to search the dll's version info table for a specific string resource - 

\StringFileInfo\040904E4\AsyncDrawingSupport

value of “1” indicates support, no entry or a value of "0" indicates no support.
[Tracking Requested - why for this release]: We should get this into 48, let's track for that
tracking-firefox48: --- → ?
OK, tracking as we discussed in irc, since 48 and onwards may be affected.
status-firefox48: --- → ?
status-firefox49: --- → ?
tracking-firefox48: ? → +
tracking-firefox49: --- → +
tracking-firefox50: --- → +

Updated

2 years ago
Priority: -- → P1
Blocks: 558448
(Assignee)

Comment 3

2 years ago
Created attachment 8773281 [details] [diff] [review]
patch
(Assignee)

Comment 4

2 years ago
Created attachment 8773283 [details] [diff] [review]
patch
Attachment #8773281 - Attachment is obsolete: true
(Assignee)

Comment 5

2 years ago
Created attachment 8773296 [details] [diff] [review]
patch

We need to check flash dlls for a string that may or may not be present regardless of version. We have access to the dll when we refresh pluginreg.dat and when we load the library to instantiate an instance. I'm purposely avoiding attempting to store this information in pluginreg.dat or propagate it around with the plugin tag info to keep the patch simple for uplift.
Attachment #8773283 - Attachment is obsolete: true
Attachment #8773296 - Flags: review?(aklotz)
Comment on attachment 8773296 [details] [diff] [review]
patch

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

::: dom/plugins/base/nsPluginsDir.h
@@ +33,5 @@
>  	char** fExtensionArray;
>  	char* fFileName;
>  	char* fFullPath;
>  	char* fVersion;
> +  bool fSupportsAsyncRender;

ws
(Assignee)

Updated

2 years ago
status-firefox48: ? → wontfix
status-firefox49: ? → affected
(Assignee)

Updated

2 years ago
Blocks: 1271398

Updated

2 years ago
Attachment #8773296 - Flags: review?(aklotz) → review+
Will IME work if Flash supports async rendering?
Flags: needinfo?(jmathies)
(Assignee)

Comment 9

2 years ago
Created attachment 8774533 [details] [diff] [review]
patch

minor touch up on some ifdef'ing here which broke on linux.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f7be7fd493560cea62192acabbc2bcb766b3378
Attachment #8773296 - Attachment is obsolete: true
Flags: needinfo?(jmathies)
Attachment #8774533 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 10

2 years ago
(In reply to Masatoshi Kimura [:emk] from comment #8)
> Will IME work if Flash supports async rendering?

I assume so. I have the beta installed and I can add an IME, can you give me some guidance on testing?
Flags: needinfo?(VYV03354)
(In reply to Jim Mathies [:jimm] from comment #10)
> I assume so. I have the beta installed and I can add an IME, can you give me
> some guidance on testing?

1. Install Japanese language pack to add an IME.
2. Open <https://emk.name/test/swftxt.html>.
3. Press Alt+[`] (if US keyboard) on the Flash text box.
4. Type [a] and [Enter]. "あ" will be displayed if the IME works.
Flags: needinfo?(VYV03354)
1.5. Switch the keyboard layout to Japanese.

Comment 13

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0474eda33487
Disable windowless workaround if we detect a flash library that support async rendering. r=aklotz
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0474eda33487
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Assignee)

Updated

2 years ago
Flags: needinfo?(jmathies)
(Assignee)

Comment 15

2 years ago
Comment on attachment 8774533 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]:
Enable new async drawing feature available in flash rev 23.
[User impact if declined]:
windowed plugins that could be removed from our content view remain. (which sucks!)
[Describe test coverage new/current, TreeHerder]:
Currently on nightly, tested by me, I've also asked Adobe to take nightly for a spin to check for issues.
[Risks and why]:
Enables a new drawing mode so there is some risk of fallout. The new Flash feature has been in development for around seven months.
[String/UUID change made/needed]:
none.
Flags: needinfo?(jmathies)
Attachment #8774533 - Flags: approval-mozilla-aurora?
Neither wmode=window nor wmode=opaque worked for me with Nightly+Flash Player 23.0.0.111 beta. Even ASCII input did not work. Only wmode=transparent worked. Is this a beta issue?
(In reply to Masatoshi Kimura [:emk] from comment #16)
> Neither wmode=window nor wmode=opaque worked for me with Nightly+Flash
> Player 23.0.0.111 beta. Even ASCII input did not work. Only
> wmode=transparent worked. Is this a beta issue?

It was bug 1289859.
(Assignee)

Comment 18

2 years ago
(In reply to Masatoshi Kimura [:emk] from comment #17)
> (In reply to Masatoshi Kimura [:emk] from comment #16)
> > Neither wmode=window nor wmode=opaque worked for me with Nightly+Flash
> > Player 23.0.0.111 beta. Even ASCII input did not work. Only
> > wmode=transparent worked. Is this a beta issue?
> 
> It was bug 1289859.

Thanks, I've requesting tracking on that for 49 so it gets uplifted.
Comment on attachment 8774533 [details] [diff] [review]
patch

This patch fixes async drawing feature flash player issue. Take it in aurora.
Attachment #8774533 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 20

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/698bdd627b48
status-firefox49: affected → fixed
(Assignee)

Updated

2 years ago
Depends on: 1294213
(Assignee)

Updated

2 years ago
Depends on: 1305135
Depends on: 1305595
(Assignee)

Updated

2 years ago
Depends on: 1312530

Comment 21

2 years ago
It looks as if this fix slipped from getting included to Firefox 50.0 64-bit release. Marked that down as https://bugzilla.mozilla.org/show_bug.cgi?id=1317995.
(In reply to Jukka Jylänki from comment #21)
> It looks as if this fix slipped from getting included to Firefox 50.0 64-bit
> release. Marked that down as
> https://bugzilla.mozilla.org/show_bug.cgi?id=1317995.

Or the 64-bit specific code in this patch is the difference.
Flags: needinfo?(jmathies)
(Assignee)

Comment 23

2 years ago
(In reply to Jukka Jylänki from comment #21)
> It looks as if this fix slipped from getting included to Firefox 50.0 64-bit
> release. Marked that down as
> https://bugzilla.mozilla.org/show_bug.cgi?id=1317995.

Here's the code - 

https://dxr.mozilla.org/mozilla-release/rev/e5e59c346966c94f21ff5bf9033b1f08b30ed077/dom/plugins/ipc/PluginModuleParent.cpp#2717

On 64-bit firefox checks the async pref (which is false) then checks for a wmode value that isn't "transparent" and convert it if needed.
Flags: needinfo?(jmathies)
You need to log in before you can comment on or make changes to this bug.