Closed Bug 604303 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: shaver, Assigned: jimm)

References

Details

(Keywords: regression)

Attachments

(4 files, 4 obsolete files)

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.)
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.
blocking2.0: --- → ?
Dammit.
Assignee: nobody → jmathies
blocking2.0: ? → betaN+
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.
(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.
There seems to be a focus component to this bug, it is a little tricky. Wonder if bug 595132 is related.
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.
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.
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.
Strangely I can't even get a right click menu in 2010-08-28 builds.
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.
Hulu's main sliding video banner seems to reproduce reliably.
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.
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.)
(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.
Duplicate of this bug: 612024
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
Blocks: 130078
Keywords: regression
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.
Duplicate of this bug: 616022
Summary: can't interact with Flash internal popups (local storage, AIR install) → [adbe 2742652] can't interact with Flash internal popups (local storage, AIR install)
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?
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).
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.
(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.)
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
(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?
Attached patch fix v.1 (obsolete) — Splinter Review
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!
Attached patch quirks patch (obsolete) — Splinter Review
Attachment #495166 - Attachment is obsolete: true
Attached patch fix patch (obsolete) — Splinter Review
Attached patch fix patch (obsolete) — Splinter Review
bug fix for quirks constants.
Attachment #495176 - Attachment is obsolete: true
Attachment #495174 - Attachment is obsolete: true
Attachment #495180 - Attachment is obsolete: true
Attachment #495291 - Flags: review?(benjamin)
Attached patch expose patch v.1Splinter Review
Not really related, but as long as I'm moving quirks, I wanted to integrate this linux flash one-off into these changes.
Attachment #495294 - Flags: review?(karlt)
Attachment #495295 - Flags: review?(bent.mozilla)
(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 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?
Attachment #495295 - Flags: review?(bent.mozilla) → review+
(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 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.
Attachment #495294 - Flags: review?(karlt) → review+
(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.
(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
Attachment #495291 - Flags: review?(benjamin) → review+
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!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 627614
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??
You need to log in before you can comment on or make changes to this bug.