2.53 KB, application/x-shockwave-flash
459 bytes, text/html
684 bytes, text/html
4.84 KB, patch
|Details | Diff | Splinter Review|
6.80 KB, patch
|Details | Diff | Splinter Review|
Firefox 2 and Flash: regression with caret animation in text fields Something changed between Firefox 1.5 and 2 that causes Flash text field carets to no longer animate. For example, go to site: http://www.permadi.com/tutorial/flashTextUnderline/index.html Scroll down the page to the blue rectangle titled "Input Text Field" click on the "Type something" text in blue you'll notice that the flash caret doesn't appear however, selecting one or more characters causes character selection to show up but clicking to show an insertion bar will never appear. This same website works fine in FF 1.5 I don't have a regression range to go along with this bug :(
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061023 Minefield/3.0a1 1.9a1_2006072814 had still a cursor but not 1.9a1_2006072816.
(In reply to comment #1) > 1.9a1_2006072814 had still a cursor but not 1.9a1_2006072816 Seems to me like bug 271442 is the most likely cause, then.
Note, that if you click on text in the wmode=opaque input text field, then click anywhere in the first flash movie, then the caret appears in the second one. Clicking elsewhere on the page does not have this effect.
Created attachment 243368 [details] Modified testcase with 4 movies This definitely seems to be a problem with Flash figuring out which movie has the focus. You can click on test in either opaque movie and the caret will only be shown (and indeed will be shown) when the focus is on the second non-opaque movie. I'm just hypothesizing, but if this is caused by our now returning the correct enclosing HWND for nsPluginInstancePeerVariable_NetscapeWindow rather than the document window (which we do so that non windowed flash in scrolling/floating DIVs can actually be clicked on at all), then we have the issue of the variable being used for two incompatible things. This could presumably be fixed in flash without adding another variable, depending on what logic they are currently using to figure out who has the focus in the opaque case. In the short term what we try to could do is to return the doccument window instead of the containing window if the two have the same origin (that way anything that worked before would work both for clicking and for caret), and the containing window in any other case (because presumably being able to interact with the movie at all is more important than it showing the caret). That way we are still better than what we had before, and can figure out how to get Flash to both see correct coordinates and show a caret in an absolutely positioned or scrolling DIV.
(sorry meant to finish the last paragraph with "later")
That sounds good. Thanks for following up
Note, since it isn't explicitly stated here, this only affects windowless flash plugins on Windows.
Note can someone confirm that prior to the 271442 fix, there are actually problems with multiple movies (e.g. looking at the 4 movie test case). Click around a little, and you end up with carets flashing in both opaque windows.
In fact event the two movie test case shows problems on Firefox 22.214.171.124 also: If you click in the first movie; caret AOK Then click in the second movie; caret AOK Click back in the first movie; caret AOK Click in the second movie; NO CARET! I bring this up because the new tentative patch I made does fix 271442 AND seems to preserve this existing broken behaviour - it seems the Flash plugin is generally a bit confused as to when to show the caret for windowless controls irrespective of what window we give it. As said before, if we include the new fix then it should be possible for an Adobe fix to fix the problem correctly later, but it does make our definition of the netscapeWindow a bit convoluted; i.e. "It's the containing document's window unless you are in a windowless plugin whose containing window's origin differs from that of the document's window in which case it is the containing window" As I said this definition allows us to return the document window as before when it doesn't matter, but the containing window in the cases where it is necessary to correctly calculate mouse event coordinates. I guess making as many things work as possible is the best solution for now, though presumably with an Adobe fix we could always return the containing window, and not do any code change on the Firefox side.
Clearing the blocking 1.8 flag since we've, uh, shipped .. :)
good point ;-) Is there anyone at Adobe/Macromedia in the loop; the point being that while I do have a fix for the regression; how it currently is (in Firefox 2.0) is arguably a more sensible definition for the HWND parameter. Given that focus does seem to be broken somewhat for windowless flash plugins anyway (see my multiple flash movie posts above), if there was any hope of a patch to Flash, then presumably this issue would go away and we wouldn't have to put more Flash workaround code in here.
I think we should take the regression fix, even if it's making things more complicated, and we can simplify things later if we get buy-in from Adobe.
ok; i'll resync to latest Fx2.0 code do some more testing then post a patch
Created attachment 244134 [details] [diff] [review] Potential 1.8 patch to resolve this issue implementation as described above
Yes, please put your workaround in place, we're not in a position to release a patch for this any time soon. When I do get a chance to work on this, is there a new NPVERS_ value that I can check to know that I'm in Firefox 2 or any other browser that supports the new code? Not suggesting we create one just for this, rather I'm hoping one is already in place that I can piggyback on.
Perhaps we could add two new properties (in addition to the existing "netscape window"... one being "document window" which always returns that one being "enclosing window" which always returns the closest window. Of course all this is just a stop-gap until (as I'm guessing we do for Fx 3.0, we need a richer windowless interface).
(In reply to comment #18) > When I do get a chance to work on this, is there a new NPVERS_ value that I can > check to know that I'm in Firefox 2 or any other browser that supports the new > code? Not suggesting we create one just for this, rather I'm hoping one is > already in place that I can piggyback on. I don't think there's a version you can reliably piggyback on (might work for now but who knows what never versions of other browsers will do to update their plugin support etc). But a plugin can get at the browsers user agent string through NPN_UserAgen(), and that should be usable (even tho not necessarily 100% guaranteed to always represent reality).
Comment on attachment 244134 [details] [diff] [review] Potential 1.8 patch to resolve this issue I think we can simplify this by creating a new nonvirtual nsIFrame method: nsIFrame* GetAncestorWithWidget() const; simliar to GetAncestorWithView which already exists in nsIFrame.h/nsFrame.cpp. Then your patch in nsObjectFrame just has to 1) call GetAncestorWithWidget on the current frame 2) Call ancestorWithWidget->GetOffsetTo(GetPresContext()->PresShell()->GetRootFrame()) 3) Return the document widget if the offset is (0,0), otherwise return ancestorWithWidget->GetWindow()
OK; I'll make the change - I figured there probably would be some utility functions to help, but I'm not too familiar with the code... is there a good site for a source code XRef?
No problem, that's why I'm here. http://lxr.mozilla.org
We do want to get this as soon as possible, but with the absence of a reviewed patch, we're not going to block on this for 126.96.36.199
Created attachment 246641 [details] [diff] [review] Propose fix I tried coding this using a new nsIFrame method that walked up the frame tree looking for those with a frame with a widget, but that didn't seem to work as I expected... while figuring out why, I discovered some existing methods which do the trick anyway.
To clarify my previous post: what I meant to say is that in implementing the suggested getAncestorWithWidget, I was climbing the frame tree looking for a frame with a view with a widget. For whatever reason (I'm not exactly sure how the frame structure and view structure differ in the case of scrollable views) this didn't appear to be equivalent to climbing the frame tree to the first view and then the view tree to the first view with a widget. In any case, when considering doing the later, I decided to just find the view for the nearest widget to the plugin, and check its offset from the root view for the view manager (which and correct me if I'm wrong) would have the same origin as the containing document's widget. This didn't require adding any new methods.
Comment on attachment 246641 [details] [diff] [review] Propose fix excellent!
Checked into trunk.
Comment on attachment 246641 [details] [diff] [review] Propose fix We may want this on branch --- not sure.
Comment on attachment 246641 [details] [diff] [review] Propose fix Approved for 1.8 branch, a=jay for drivers.
Fixed on branch.
Confirm Verified Fixed for 188.8.131.52 and trunk - tested with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:184.108.40.206pre) Gecko/2007011004 BonEcho/220.127.116.11pre and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a2pre) Gecko/2007011404 Minefield/3.0a2pre