604303 - [adbe 2742652] can't interact with Flash internal popups (local storage, AIR install)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I get this when Flash pops up an "internal dialog" about installing AIR (I
know, I know): click the "Download and Install" button in the middle of the
page on http://www.rdio.com/#/apps/desktop/ .

(From bug 601511.)

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

This is at *least* betaN+, maybe beta8+, IMO: if a site requires flash local storage to work (as rdio does), then you can't grant it permission and use the app.

Comment 2

[Benjamin Smedberg]

Dammit.

Comment 3

[Jim Mathies [:jimm]]

worked 8/27, fails 8/29. That points to 130078.

tn, isn't there a mouse coordinate patch posted someplace that still needs landing? I wonder if that's related here.

STR:

1) open youtube
2) right-click on the big flash ad banner, select settings (or)
click through to a movie and do the same
3) try and close the little settings window

I'm not able to interact with it post 8/27. Disabling ipc doesn't seem to help.

Comment 4

[Timothy Nikkel (:tnikkel)]

(In reply to comment #3)
> tn, isn't there a mouse coordinate patch posted someplace that still needs
> landing? I wonder if that's related here.

The only one I can think of is bug 596600, but it doesn't have a patch yet and I don't know if its even related to this bug.

Comment 5

[Timothy Nikkel (:tnikkel)]

There seems to be a focus component to this bug, it is a little tricky. Wonder if bug 595132 is related.

Comment 6

[Timothy Nikkel (:tnikkel)]

I hope the youtube STR represent the same problem as I don't have an rdio account.

It seems like the setting dialog has a delay of a second or two before it accepts mouse events. And I can reproduce that in the 2010-08-27 nightly.

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Yeah, the rdio thing is just a different Flash-internal dialog.  You can get an rdio trial, though, and if you need it extended for testing I can arrange that.

Comment 8

[Timothy Nikkel (:tnikkel)]

So after some more testing I think I'm seeing the same thing both pre-130078 and post-130078, and even in 3.6: just that the settings dialog ignores mouse events for a second or two and then works.

Comment 9

[Timothy Nikkel (:tnikkel)]

shaver pointed me at http://www.macromedia.com/support/documentation/en/flashplayer/help/settings_manager03.html#117498 and on that page I can reproduce.

Comment 10

[Timothy Nikkel (:tnikkel)]

Strangely I can't even get a right click menu in 2010-08-28 builds.

Comment 11

[Jim Mathies [:jimm]]

It seems rather random, I just tried again on yesterday's nightly and I can interact with it again on a youtube video.

I also don't have any problems on a stand alone video embedded in a simple web page.

Comment 12

[Jim Mathies [:jimm]]

Hulu's main sliding video banner seems to reproduce reliably.

Comment 13

[Jim Mathies [:jimm]]

Attachment: hulu windowless testcase

Comment 14

[Jim Mathies [:jimm]]

I've can confirm we are sending the same event coordinates to flash regardless of whether these little popups are visible. In the main flash view port our events are handled correctly, in these little "popups" they aren't. Almost seems like a bug in flash since the coords are correct. 

The event coords we send are relative to the window flash sits in, so for example in the test case I attached, the upper left hand corner has coords of 10, 122. We don't appear to be doing anything wrong here, flash doesn't seem to like the fact that we removed the widget it used to sit in.

Comment 15

[Timothy Nikkel (:tnikkel)]

Looks like it is a Flash bug that has been affecting Opera for a while now: https://bugs.adobe.com/jira/browse/FP-4867

(Credit to Rafal for a comment on roc's blog post here http://weblogs.mozillazine.org/roc/archives/2010/10/bye_bye_hwnds.html for pointing this out.)

Comment 16

[Jim Mathies [:jimm]]

(In reply to comment #15)
> Looks like it is a Flash bug that has been affecting Opera for a while now:
> https://bugs.adobe.com/jira/browse/FP-4867
> 
> (Credit to Rafal for a comment on roc's blog post here
> http://weblogs.mozillazine.org/roc/archives/2010/10/bye_bye_hwnds.html for
> pointing this out.)

Great, thanks for the link. I've been digging into this again today, quite frustrating.

Comment 18

[Alice0775 White]

See https://bugzilla.mozilla.org/show_bug.cgi?id=612024#c2

The following cset causes the problem.
7804b5cf4313    Robert O'Callahan — Bug 130078. Part 2. Remove widgets from all
subframes. r=dbaron

Comment 19

[Jim Mathies [:jimm]]

cc'ing Josh. 

Josh, bsmedberg mentioned you meet with the flash folks on occasionally. Curious if we might bring this one up. AFAICT this isn't a bug in our code, we hand the right coordinates in. Digging through flash assembly hasn't turned up much either. Someone with the src should be able to diagnose this fairly easily.

Comment 21

[Jim Mathies [:jimm]]

Summary this far:

In bug 130078 we removed all child widget from the main browser view. There's currently only one Window window now at the top level.

I've confirmed we're sending the same event coordinates to flash regardless of whether or not these little internal dialogs are displayed in flash.

From poking at flash, the only system api they seem to call is WindowFromPoint on mouse down, this succeeds and returns the top level browser window. There are also some calls to IsVisible and GetWindowInfo.

Overall I'm still stumped on a fix.

Stepping through flash's source should disclose the problem in short order. Curious of some of the folks from Adobe now cc'd in might be able to help us track this down?

Comment 22

[Emanuel Hoogeveen [:ehoogeveen]]

If reliable STR are still needed, I always seem to get this on justin.tv, e.g. http://www.justin.tv/spoonyone (regardless of whether the channel is live, zoom level or d2d being enabled or disabled).

Comment 23

[Rafał]

As I have mentioned in Adobe bug, the problem is that Flash does security checks when in settings mode. THe problem is that it does the check according to window rect while it should do them according to client rect. It can't be fixed in browser because we can't make client rect and window rect the same for main window.

If you wanna play a hacker then break in flash where it calls GetWindowInfo and change the code where it accesses window rect to point 16 bytes forward (size of the RECT structure). It will work then.

Comment 24

[Jim Mathies [:jimm]]

(In reply to comment #23)
> As I have mentioned in Adobe bug, the problem is that Flash does security
> checks when in settings mode. THe problem is that it does the check according
> to window rect while it should do them according to client rect. It can't be
> fixed in browser because we can't make client rect and window rect the same for
> main window.

Hey Rafał, do you have any idea what security checks flash does, and what it is looking for in the results of GetWindowInfo? (We can trap the calls GetWindowInfo if we need to and modify the results.)

Comment 25

[Rafał]

As I said, it checks window rect while it should check client rect.
See http://msdn.microsoft.com/en-us/library/ms632610(v=VS.85).aspx

Comment 26

[Jim Mathies [:jimm]]

(In reply to comment #25)
> As I said, it checks window rect while it should check client rect.
> See http://msdn.microsoft.com/en-us/library/ms632610(v=VS.85).aspx

Right, I get that. I don't understand why flash would be looking at either of these rects for security purposes. Any ideas why they do this?

Comment 27

[Jim Mathies [:jimm]]

Attachment: fix v.1

This is just a prototype. I need to integrate this in with quirks modes and to do that I need to dust off some patches that move quirks into the module. 

I still don't know why flash would do this check. Regardless it's our window so I suppose we can return whatever information we want about it. 

Hat tip to Rafał on the GetWindowInfo tip!

Comment 28

[Jim Mathies [:jimm]]

Attachment: quirks patch

Comment 29

[Jim Mathies [:jimm]]

Attachment: fix patch

Comment 30

[Jim Mathies [:jimm]]

Attachment: fix patch

bug fix for quirks constants.

Comment 31

[Jim Mathies [:jimm]]

Attachment: move quirks to module v.1

Comment 32

[Jim Mathies [:jimm]]

Attachment: expose patch v.1

Not really related, but as long as I'm moving quirks, I wanted to integrate this linux flash one-off into these changes.

Comment 33

[Jim Mathies [:jimm]]

Attachment: flash get window info patch v.1

Comment 34

[Jim Mathies [:jimm]]

(In reply to comment #33)
> Created attachment 495295 [details] [diff] [review]
> flash get window info patch v.1

Just noticed, kManageWindowInfoProperty up top is a copy paste left over, it's not needed and will be removed.

Comment 35

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 495295 [details] [diff] [review]
flash get window info patch v.1

This looks ok, but shouldn't we remove the hook when this thing goes away?

Comment 36

[Jim Mathies [:jimm]]

(In reply to comment #35)
> Comment on attachment 495295 [details] [diff] [review]
> flash get window info patch v.1
> 
> This looks ok, but shouldn't we remove the hook when this thing goes away?

I'm considering tying this to a major version of flash so that it'll disable on a future version. Is that what you had in mind?

Comment 37

[Karl Tomlinson (:karlt)]

Comment on attachment 495294 [details] [diff] [review]
expose patch v.1

LGTM thanks.
I assume that NPPVpluginDescriptionString can be requested without an instance, but that can be dealt with separately.

Comment 38

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #36)
> Is that what you had in mind?

No, sorry, I was unclear. I was wondering if we should unhook the dll when the PluginModuleChild is destroyed.

Comment 39

[Jim Mathies [:jimm]]

(In reply to comment #38)
> (In reply to comment #36)
> > Is that what you had in mind?
> 
> No, sorry, I was unclear. I was wondering if we should unhook the dll when the
> PluginModuleChild is destroyed.

Ah, there's no way to unhook. It's a one way ticket!

http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsWindowsDllInterceptor.h

Comment 40

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/b61efde547aa
http://hg.mozilla.org/mozilla-central/rev/038591612c97
http://hg.mozilla.org/mozilla-central/rev/fb8ad78374df

If flash folks ever get around to fixing this, please let us know in advance!

Comment 41

[Sander Kruger]

If this is a Flash bug and not a Firefox bug, then why does the bug - with the exact same Flash Player version - occur only in Firefox 4.0 and not in 3.6??