If some flash content has focus and you click on another firefox window, you will not be able to type into that window. The only way to get the keyboard to type into the other window is to first put firefox in the background (click on the desktop or other application) and then click on the window you want to bring to the foreground. To reproduce: 1. Open two windows. 2. In one window open a page that has a form. 3. In the other window, open anything that has flash content. For example http://video.google.com/ 4. Click on the flash content to give it focus. 5. Click on your other window and try to type into the form input, the location bar, or the search field. You will not be able to type anything. 6. Click on the desktop and then click on the window again. You will now be able to type. This problem does not happen if you are switching tabs. This problem does not happen for java applets. It must be flash and it must be switching windows.
I should have mentioned that I am using 2.0RC1 Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20060918 Firefox/2.0
This bug may be a dup of bug 345010.
I was able to confirm this with FF 2.0 RC1 on Mac OS X 10.3.9, using either one of the videos at http://video.google.com/ or the Flash object at http://www.rogerdean.com/ (at the top of the page).
*** Bug 356365 has been marked as a duplicate of this bug. ***
This bug really needs attention before 2.0 ships. Perhaps if it didn't have severity: normal, Jake?
Quite frankly, I doubt that it's possible to resolve this bug before 2.0 ships -- since it's likely that a fix for bug 318139 will be required, and will involve changes both to Mozilla.org browsers and to the Flash plugin. But it's <a href="mailto:email@example.com">Mark Mentovai</a> and <a href="mailto:firstname.lastname@example.org">Michelle Sintov</a> who know most about this.
I have seen reports of people hitting a variant of this bug in reporter and have hit it myself. It is particularly frustrating for users because when they think they can't login, they abandon the browser for another browser. The site that I have seen it on recently is http://www.sirius.com/. If I click on "Listen Online" and then try to fill in my login information, I can't type anything in the space. Sometimes after some period of time passes, I am able to fill in the form.
I just tried http://www.sirius.com/ and was unable to enter any text in the window popped up by clicking "Listen Online" -- until I clicked on the desktop and again on that window. So yes, Marcia, it does seem like you've seen this bug. I'm curious what Mark has to say, but I do think this will take a while to fix (since it will likely require changes to the Flash plugin as well as to the Mozilla.org browsers).
I dont know the details of how the code works, but this problem only appears on 2.0. When I switch to 1.5, I never encounter this problem. So, I have a hard time believing that this is a problem with Flash or that it is difficult to fix. Something changed between 1.5 and 2.0 that caused this to happen.
If this is a dup of bug 345010 (which it very likely is), the fact that keyboard input is only blocked FF 2.0 (and not in FF 1.5) is due to the fact that Carbon-based browsers on the 1.8 and 1.8.1 branches (but not the 1.8.0 branch) have been getting their their input via TSM (from the kUnicodeNotFromInputMethod Apple event), since the fix for bug 337199 was landed. (I believe this fix was also landed on the trunk.) The bad interaction between the Flash plugin and Mozilla.org's Carbon-based browsers (involving those programs' conflicting use of the "TSM Document") still happens on other branches ... it just doesn't (usually) result in keyboard input being blocked. The symptoms can include crashes (as described at bug 318139). Note: I understand the problem(s) pretty well, but I can't fix them. On the Mozilla.org side, I suspect only Mark Mentovai could do so. (Though, as I've mentioned a couple of times above, even he might not be able to fix it/them without same changes to the Flash plugin.) Mark? :-)
I finally had a chance to test the effect of this bug's URLs on the current "TSM document" -- and they didn't have any (bad) effect. So I was wrong -- this bug _isn't_ a dup of bug 345010, or (presumably) related in any way to bug 318139. I can't begin to say what _is_ the cause of this bug. I've only been able to reproduce it with Flash plugins, so I don't think it's a straightforward keyboard focus issue. (I also tried the QuickTime, PDF and Shockwave plugins.)
nominating to get this on the radar. A bad user experience on the Mac end, especially in light of the fact it isn't present on 1.5.0.x. Even if clicking outside of the browser window allows you to enter form information, users are not going to know to do that. Also able to reproduce this on www.zoomzoomlive.com. Click on results/photos and try to login to see what happens.
changing flags to indicate I want to block for the next release. sorry for the bugspam.
Marcia showed me the problem on her mac (I'm not sure the browser/os/flash versions she had), but for For me, results/photos on https://www.zoomzoomlive.com/index.htm works with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1) Gecko/20061023 BonEcho/2.0 Shockwave Flash File name: Flash Player.plugin Shockwave Flash 9.0 r20 Mac OS X 10.4.8 Mac Intel
(In reply to comment #14) I see the problem every time, with exactly the same software as you (except that I'm using FF 2.0 RC3), following the procedure in comment #0 and using http://www.sirius.com/ (as per comment #8).
(Following up comment #11) > I finally had a chance to test the effect of this bug's URLs on the > current "TSM document" -- and they didn't have any (bad) effect. Wrong again :-( This bug _is_ caused by problems with the "TSM document" -- just not exactly the same ones reported at bug 345010 and bug 318139. And it _is_ related to those two bugs, though it isn't a dup. As described at bug 318139 comment #0 and bug 345010 comment #0 (first two paragraphs), the Flash plugin saves and restores the TSM document for the browser window that it's running in. But sometimes it restores the wrong one. That's what's happening here. But unlike the situations reported at those other two bugs, in this case the "wrong TSM document" is still valid -- so you don't see crashes (as per bug 318139), and the browser's current TSM document doesn't get NULLed (as per bug 345010). Here's what happens when you follow the steps in comment #0. I'll call the "one window" of step 2 "window A", and the "other window" of step 3 "window B". And let's say window A's TSM document is 0xaaaaaaaa, while window B's is 0xbbbbbbbb. For keyboard input to work properly in Mozilla.org Carbon-based browsers with the fix for bug 337199, the "current TSM document" needs to be set to a given browser window's TSM document while the keyboard focus is in one of that window's text input fields. When, in step 4, you click on the flash content to give it focus, the Flash plugin saves the then current TSM document (0xbbbbbbbb, window B's TSM document) and makes its own TSM document current (call it 0x12345678). But when in step 5 you click on window A, the Flash plugin "restores" window B's TSM document (makes it the current TSM document) -- i.e. it restores it incorrectly. Keyboard input won't work in window A unless it's own TSM document is the current one. Clicking on the desktop and again on window A (as per step 6) makes window A's TSM document current -- so keyboard input starts working again in window A. (By the way, this problem can't be fixed by simply backing out the fix for bug 337199 -- Chinese, Japanese and Korean input would still be effected by this bug.)
steven, my mistake. if I follow the steps in comment #0, I can reproduce this bug with the latest FX2 using https://www.zoomzoomlive.com/index.htm my mistake was I didn't realize this bug was about one browser window (with flash) preventing you from typing into a text field on another browser window. I thought this bug was about not being able to type into the "zoomzoomlive" flash app, which is what marcia was seeing. marcia, are you able to reproduce this bug? is your zoomzoomlive bug something else?
removing ispike's dupme, as I don't think is a dup (please correct me if I'm wrong). adding [Fx 220.127.116.11] so that we can get it on the radar
*** Bug 358940 has been marked as a duplicate of this bug. ***
I did notice today that when I go to www.sirius.com and click on the "Listen Online" link, I get this in the Error Console: Error: [Exception... "'Permission denied to get property XULElement.accessKey' when calling method: [nsIDOMXULLabelElement::accessKey]" nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)" location: "JS frame :: http://cdn.sirius.com/c/scripts/common.js :: launchPlayer :: line 77" data: no] Source File: http://cdn.sirius.com/c/scripts/common.js Line: 77
I have unfortunately run across this bug too, and my company is about to launch a site that depends on Flash launching a popup window that contains a form that users must fill out. So needless to say I'm kinda freaking out, as the site goes live in 4 days. Is there a work-around out there? I would LOVE to see the priority of this bug go up as well. Thanks all.
Not realistic to block 18.104.22.168 for this since there's not been any progress. Really want it, though.
*** Bug 354952 has been marked as a duplicate of this bug. ***
11 years ago
update: bret reckard sees this bug when using yahoo sports, which pops up a window that has a flash ap.
Is this bug only seen in Firefox 2, _not_ trunk?
It _did_ happen on the trunk, but stopped happening when the switch to Cocoa widgets was landed (on 2006-09-28, according to Josh Aas's blog http://weblogs.mozillazine.org/josh/archives/2006/09/). (It happens with Minefield 2006-09-27-trunk, but not with the next available full trunk build, 2006-10-10-04.) It doesn't happen in Camino (1.0.3 or on the trunk).
Is this the same as bug 363616? (Not sure.) Interested in hearing the resolution to this.
(In reply to comment #25) > Is this bug only seen in Firefox 2, _not_ trunk? > I also get it in an Embed project we're working on, using the ActiveX wrapper that (I think) is based on 1.7.2.
> Is this the same as bug 363616? I doubt it. But bug 363616 is very ill defined ... so it's difficult to be sure.
over to nobody, as mark doesn't have cycles for this. if someone does have cycles, steven's comment #16 looks like a good place to start.
Clearing blocking flag, but would be nice to get this fixed. Assigning to Josh Aas for more investigation.
Created attachment 250807 [details] [diff] [review] Fix for this bug 355071 and bug 345010 Here's a fix for this bug (355071) and bug 345010. It's a patch against the Firefox 22.214.171.124 source, and should still apply without offsets to the 1.8.1 branch (the relevant files haven't changed since 126.96.36.199 came out). Once this patch is landed (or something like it), plugins that (like the Flash plugin) do IME input (and set their own TSM documents) won't need to worry about setting the current TSM document back (to one the browser can use). In fact it won't even be a problem if these plugins set the TSM document incorrectly (as the Flash plugin now does for this bug (355071) and bug 345010). The basic logic is very simple: On every raw key down event, I check if the keyboard focus is in a plugin or not. If it isn't and the current TSM document isn't the browser's TSM document for the current page, I activate the browser's TSM document (for the current page). But it was a real challenge figuring out how to tell when a plugin has the keyboard focus. I think my patch does this reliably and efficiently, but it's still _quite_ complex. If anyone knows a simpler method, please pass it along.
Created attachment 250893 [details] [diff] [review] Revised fix (more optimizations) It turns out that the "focused widget" returned by mEventDispatchHandler->GetActive() is usually (always?) a top-level widget -- which means that calls to WidgetContentIsPlugin() using this widget can cause the entire widget hierarchy to be browsed. So I've optimized away unnecessary calls to WidgetContentIsPlugin().
Created attachment 251000 [details] [diff] [review] Revised fix (works with shared build) Here's yet another revision of my patch. I found that I needed to fiddle with Makefile.in to get the patch to compile in a shared-libary build, and I made a couple of other minor changes.
Josh, if you don't have time for this, or think someone else should review my patch, please suggest a name. If possible I'd like to get the patch into Firefox 188.8.131.52. And I don't want to still be making changes to it at the last minute :-)
Comment on attachment 251000 [details] [diff] [review] Revised fix (works with shared build) I'd like to have roc look at this if he's willing to. If there is a faster way to do some of this he'd probably know.
Created attachment 251396 [details] [diff] [review] Revised fix (simpler) I dug around in nsViewManager and nsPresShell, and found a simpler way to detect when a plugin has the keyboard focus. It may not be any faster (nsFrameManager::GetPrimaryFrameFor(nsIContent* aContent) still occasionally needs to browse the entire nsIContent hierarchy), but I'm sure it's better in the long run (GetPrimaryFrameFor() seems to lie at the heart of the event-processing infrastructure, which means that a lot of attention is spent on keeping it optimized).
Change to more accurate summary.
roc, if you can, please review this patch sometime in the next few days ... or please suggest someone else for the job :-) If possible I'd like to get this patch into Firefox 184.108.40.206. This bug was nominated (as a blocker) and rejected a couple of times ... but that's (presumably) because a patch didn't yet exist.
I'm not in a position to build right now, but as I seem to have the problem in FF 220.127.116.11 like 100% of the time, I'd be happy to test a release dll for you...
(In reply to comment #41) Unfortunately, I can't give you anything to test until this patch gets landed (on the mozilla1.8 branch).
+ifdef BUILD_SHARED_LIBS +SHARED_LIBRARY_LIBS = \ + $(DIST)/lib/libxpwidgets_s.a \ + $(DIST)/lib/libgkview_s.a \ + $(NULL) +else This is bad. You're linking in an extra copy of views here with no guarantee that things will actually work properly with two copies. When a plugin is focused, aren't there OS-level focus notifications that you can observe to detect that we're in a plugin? If you just know which widget is focused, we can easily add an API to nsIWidget to determine whether the widget is for a plugin or not.
> aren't there OS-level focus notifications that you can observe to > detect that we're in a plugin? I don't think so ... but I'm looking into it. > You're linking in an extra copy of views As best I can tell, there's just one views dependency that requires me to explicitly link in libgkview_s.a (and then only when doing a shared build) -- nsIView::GetViewFor(). Do you know of a way I can get an nsIDocument from an nsIWidget without using nsIView::GetViewFor(nsIWidget)? (I can't just copy the code for nsIView::GetViewFor(nsIWidget), since that uses the private ViewWrapper class.) > If you just know which widget is focused, we can easily add an API > to nsIWidget to determine whether the widget is for a plugin or not. I _do_ have a widget ... but it seems to always be a top-level widget.
> we can easily add an API to nsIWidget to determine whether the > widget is for a plugin or not Could you add an API to get the nsIDocument that corresponds to an nsIWidget? Something like the nsIDocument * GetDocumentFor(nsIWidget *aWidget) I have in my patch?
I could, but in the future I'd like to have documents that aren't associated with widgets. I think the best thing to do is to figure out which native widget has focus somehow, then see whether that's a plugin.
> I think the best thing to do is to figure out which native widget > has focus somehow, then see whether that's a plugin. By "native widget" do you mean something like a ControlRef (a Carbon object -- something that can have the keyboard focus)? If so, I don't think it will be possible to know whether or not this corresponds to (or is inside) a plugin, using only information garnered from the OS -- the notion of "plugin" is a browser concept, not an OS concept.
Can we map from the Carbon object to an nsIWidget?
> Can we map from the Carbon object to an nsIWidget? Not as things currently stand. I could add code to do this, but it would require a hashtable or linked list. I've now found another place to look for a solution (one that doesn't use the nsIWidget hierarchy, nsIDocument, or any of that stuff): There's an nsIWidget::SetFocus() method, which is implemented in widget/src/mac/nsWindow.cpp. It doesn't currently keep track of which nsIWidget has been focused by the browser, but I could make it do so. The question is whether this method gets called whenever an nsIFrame is focused (on the nsIWidget corresponding to that nsIFrame).
Created attachment 251723 [details] [diff] [review] Fix, rev4 After beating my head against several walls for more than a day, I've discovered that it's impossible to know whether or not a plugin is focused without using the nsIDocument associated with the nsIWidget provided by mEventDispatchHandler->GetActive(). More about this below. So my current patch isn't much changed from the previous version. But I did figure out how to stop using nsIView::GetViewFor(nsIWidget). So we no longer need to link in libgkview_s.a, even when doing a shared library build. I also moved the static "utility" methods to nsToolkit -- it seemed a better place to put them. I found a clever way to associate an nsIWidget with each ControlRef created by the Mac-specific widget code (you can set an arbitrary "property" on a ControlRef). But then I found that the Control Manager GetKeyboardFocus() method (which is supposed to tell you which ControlRef is focused) always returns NULL (and these objects never receive any of the kEventClassControl "focus events"). The comment on SetKeyboardFocus() says that "keyboard focus is only available if an embedding hierarchy has been established in the focusable control's window". I assume this means that the focus methods only work (and the associated Carbon events are only sent) when your controls are HIView objects. It'd be a great deal of trouble to change to using HIView controls -- and certainly not worth doing in a dead-end branch like the mozilla1.8 branch. Then I took a look at the nsWindow::SetFocus() method. But I discovered that this method is only ever called on fairly high-level widgets (though not usually the top-level widget) -- so near the top of the widget hierarchy that, when I load a plugin into the browser page, _every widget_ contains the plugin. It took me several hours, but I finally tracked this problem down to how nsIWidget::SetFocus() is called in nsEventStateManager::SendFocusBlur() (in the "if (aEnsureWindowHasFocus)" block) -- it's called on the widget associated with the nsViewManager's root view, not on the widget associated with the relevant nsGUIEvent.
Well, roc, do you want me to make any more changes?
I don't like this approach because it's really ugly and scary-fragile. It's branch only, so I guess we can take it if we have no other choice, but I'd like to find another choice if we possibly can. What if we just set the current TSM document to our window's TSM document when our window gets focused, would that work?
> What if we just set the current TSM document to our window's TSM > document when our window gets focused, would that work? No. If the focus is in a plugin, this would break IME input in the plugin (the Flash plugin does support IME input). > it's really ugly and scary What are you talking about here? Do you think my code doesn't reliably determine when a plugin is focused? Are you worried about false negatives? False positives? Or are there cases when the TSM doc shouldn't be corrected (set to the browser's window's TSM doc) even when a plugin isn't focused?
>> What if we just set the current TSM document to our window's TSM >> document when our window gets focused, would that work? > > No. If the focus is in a plugin, this would break IME input in the > plugin (the Flash plugin does support IME input). Actually, this is already being done (in nsMacEventHandler::HandleActivateEvent()), and the Flash plugin gets around the problem of broken IME input by setting its own TSM document (when the window gets activated, I assume). But what nsMacEventHandler::HandleActivateEvent() does clearly isn't enough -- we've still got this bug and bug 345010.
Moreover (speaking of fragility and robustness), my fix has the advantage that it doesn't matter what plugins set the TSM document to -- when the focus keyboard focus is in a browser widget (as opposed to a plugin widget), the TSM document will always be set correctly (to the browser window's TSM document).
What's ugly is the fact that you need to grope around the nsIWidget hierarchy looking for a document. Including scary code like this: + if (entered > 1) + nextSibling = widgetLocal->GetNextSibling(); Why is that there? Merely because it worked, I suppose. What does it depend on? No-one knows. So if we changed the widget hierarchy a little bit, boom, his no longer works and whoever made the change will take a long time to figure out what happened. That's scary. And of course generally, widget code should not know about views, content, frames and documents. Just because code works (today) doesn't mean it's good.
oops, I understand that bit of code better now and it makes more sense. However, GetDocumentFor should be rewritten so you don't need this "entered" hack.
In particular, you should check whether aWidget has a document, and then iterate through all its children calling GetDocumentFor on each one. It's still really ugly that you to have to mess with views, content, documents etc but at least it'll make more sense. The caching you do in IsPluginFocused is dangerous. If you're very unlucky, prevWidget could be destroyed and a new widget created with the same address and passed in as aWidget. Then you would try to use a deleted document. Boom. You're also assuming that the document associated with a widget never changes, which actually isn't true. You need to get rid of that caching. (Besides, this only gets called once per input event so it is almost certainly not worth optimizing for speed).
If we were doing this on trunk I would suggest that we modify nsEventStateManager to notify the widget for a focused frame that it has been focused. I don't know why it doesn't call SetFocus, actually, but even if we don't want to call SetFocus we could call some new method that you could then track to know whether a plugin has been focused. But apparently on trunk we don't need this anyway, and we don't want to change interfaces on branch, especially cross-platform ones, so we'll go with your approach, if you make the changes I mentioned. But it's still ugly :-)
> However, GetDocumentFor should be rewritten so you don't need this > "entered" hack. I'll do that ... though there's really no need. > And of course generally, widget code should not know about views, > content, frames and documents. Generally speaking I agree with you. But in this case I have no choice. And I've seen many similar cases throughout the Mozilla.org source tree. I suppose the best solution would be an nsIView isPluginFocused() method, arranged so that (unlike nsIView::GetViewFor(nsIWidget) it doesn't have to be statically linked in. But in the meantime ... > Just because code works (today) doesn't mean it's good. When/if the widget/frame/view/document hierarchy does change, you'll also need to make _lots_ of changes elsewhere.
> In particular, you should check whether aWidget has a document, and > then iterate through all its children calling GetDocumentFor on each > one. I don't understand. I've assumed that each browser page has (at any one time) exactly one nsIDocument (or at least that whatever nsIDocument I find will lead me to a PresShell (and so forth) that will tell me which frame is focused).
> The caching you do in IsPluginFocused is dangerous. OK, I'll get rid of it.
> But apparently on trunk we don't need this anyway, Actually, I suspect that we might. Flash IME input is currently broken on the trunk, and I suspect the reason is the "fix" in nsPluginInstanceOwner::ProcessEvent() (Cocoa-widgets only) for bug 183313 that calls DeactivateTSMDocument(TSMGetActiveDocument()) on an NS_FOCUS_CONTENT event. Removing this "fix" will probably require something like what I've done here. But I also suspect that in src/mac/cocoa I'll be able to find out from the OS exactly which nsIWidget is focused.
>> In particular, you should check whether aWidget has a document, and >> then iterate through all its children calling GetDocumentFor on each >> one. > > I don't understand. I've assumed that each browser page has (at any > one time) exactly one nsIDocument (or at least that whatever > nsIDocument I find will lead me to a PresShell (and so forth) that > will tell me which frame is focused). Now I understand. You're just telling me how I should get rid of "entered".
Yeah. > And I've seen many similar cases throughout the Mozilla.org > source tree. Sure, but we try to make things better by removing badness, not make things worse by adding it.
> Sure, but we try to make things better by removing badness, not make > things worse by adding it. Sometimes "badness" isn't so bad. Sometimes the best is enemy of the good. But I'd also agree that the good should be replaced by the better when that comes along ... and oftentimes we're far too busy when that happens, and don't do it because it isn't urgent.
Created attachment 251969 [details] [diff] [review] Fix, rev5 OK, here's the next patch revision. roc, let me know what you think. There's still an outside chance my patch can make it into Firefox 18.104.22.168.
+ frame = nsnull; + content = nsnull; + document = nsnull; + view = nsToolkit::GetViewFor(widgetChild); + if (view) + frame = NS_STATIC_CAST(nsIFrame*, view->GetClientData()); + if (frame) + content = frame->GetContent(); + if (content) + document = content->GetCurrentDoc(); + if (document) + return document; + widgetChild = nextChild; Why are you doing this? This will be checked when you call GetDocumentFor recursively on widgetChild.
Created attachment 251994 [details] [diff] [review] Fix, rev6 Sorry. Here's the fix.
Comment on attachment 251994 [details] [diff] [review] Fix, rev6 ok!
What's next? Who should do the superreview? Should I ask for 22.214.171.124 approval?
If roc is willing to have his review count as sr, then you should request 126.96.36.199 approval.
Comment on attachment 251994 [details] [diff] [review] Fix, rev6 > If roc is willing to have his review count as sr On the assumption this is ok with roc, here goes.
Comment on attachment 251994 [details] [diff] [review] Fix, rev6 approved for 1.8 branch, a=dveditz for drivers
Just so that people know: I don't know how to land this patch, and even if I did I'm not sure I have the permissions.
I've landed this patch on the 1.8 branch for Firefox 188.8.131.52. Should this land on the trunk, given that widget/mac/src isn't built by default there? mozilla/widget/src/mac/nsMacEventHandler.cpp 184.108.40.206 mozilla/widget/src/mac/nsMacEventHandler.h 220.127.116.11 mozilla/widget/src/mac/nsToolkit.cpp 18.104.22.168 mozilla/widget/src/mac/nsToolkit.h 22.214.171.124
This shouldn't land on trunk.
For people who want to test my fix: The first mozilla1.8-branch nightly containing it (firefox-2007-01-22-07-mozilla1.8) has appeared at ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/
verified fixed on the 1.8 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:126.96.36.199pre) Gecko/20070125 BonEcho/188.8.131.52pre. I can't test the sirius.com site since it doesn't like the fact the build is branded Bon Echo (browser not supported...), but several other tests with variants of Comment #1 and using www.zoomzoomlive.com worked fine for me. adding fixed keyword.
Steven, why is this still open?
I probably just forgot to close it ... so I'll do that now.