If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Firefox 2 and Flash: regression with caret animation in text fields

VERIFIED FIXED

Status

()

Core
Plug-ins
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: Darin Fisher, Unassigned)

Tracking

({regression, testcase, verified1.8.1.2})

1.8 Branch
x86
Windows XP
regression, testcase, verified1.8.1.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.1 -
wanted1.8.1.x +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments)

(Reporter)

Description

11 years ago
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.
Created attachment 243358 [details]
flash for testcase
Created attachment 243359 [details]
testcase
Yeah, most likely a regression from bug 271442.
Blocks: 271442
Keywords: testcase

Comment 6

11 years ago
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.

Comment 7

11 years ago
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.

Comment 8

11 years ago
(sorry meant to finish the last paragraph with "later")
(Reporter)

Updated

11 years ago
Flags: blocking1.8.1?
Flags: blocking1.8.1.1?
That sounds good. Thanks for following up

Comment 10

11 years ago
Note, since it isn't explicitly stated here, this only affects windowless flash plugins on Windows.

Comment 11

11 years ago
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.

Comment 12

11 years ago
In fact event the two movie test case shows problems on Firefox 1.5.0.6 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 .. :)
Flags: blocking1.8.1?

Comment 14

11 years ago
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.

Comment 16

11 years ago
ok; i'll resync to latest Fx2.0 code do some more testing then post a patch

Comment 17

11 years ago
Created attachment 244134 [details] [diff] [review]
Potential 1.8 patch to resolve this issue

implementation as described above
Attachment #244134 - Flags: review?(roc)

Comment 18

11 years ago
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.

Comment 19

11 years ago
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()

Comment 22

11 years ago
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 1.8.1.1
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-

Comment 25

11 years ago
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.
Attachment #246641 - Flags: review?(roc)

Comment 26

11 years ago
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!
Attachment #246641 - Flags: superreview+
Attachment #246641 - Flags: review?(roc)
Attachment #246641 - Flags: review+
Checked into trunk.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Attachment #244134 - Flags: review?(roc)
Comment on attachment 246641 [details] [diff] [review]
Propose fix

We may want this on branch --- not sure.
Attachment #246641 - Flags: approval1.8.1.2?

Comment 30

11 years ago
Comment on attachment 246641 [details] [diff] [review]
Propose fix

Approved for 1.8 branch, a=jay for drivers.
Attachment #246641 - Flags: approval1.8.1.2? → approval1.8.1.2+
Fixed on branch.
Keywords: fixed1.8.1.2
Confirm Verified Fixed for 1.8.1.2 and trunk - tested with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/2007011004 BonEcho/2.0.0.2pre and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a2pre) Gecko/2007011404 Minefield/3.0a2pre
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.2 → verified1.8.1.2
Depends on: 371878
You need to log in before you can comment on or make changes to this bug.