446663 - Can't type in form controls with :focus{overflow:hidden}

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[dolphinling]

After typing in a form control with :focus{overflow:hidden}, on unfocusing the text is erased. See testcase in url field.

Editing the .value of the field makes it so on leaving the field, it returns to that value.

I currently can't log in to sourceforge because of this.

I assume it's a regression. Unless I can reproduce on another system, I can't test from when though unless I recompile builds from older dates (mozilla.org binaries don't work on this machine)

Maybe a dupe/symptom of Bug 422295?


Self-built mozilla-central.
Build identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1a1pre) Gecko/2008072203 Minefield/3.1a1pre

about:buildconfig

Build platform
target
x86_64-unknown-linux-gnu

Build tools
Compiler 	Version 	Compiler flags
gcc 	gcc version 4.2.3 (Gentoo 4.2.3 p1.0) 	-Wall -W -Wno-unused -Wpointer-arith -Wcast-align -W -Wno-long-long -pedantic -fno-strict-aliasing -pthread -pipe
c++ 	gcc version 4.2.3 (Gentoo 4.2.3 p1.0) 	-fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-long-long -pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe

Configure arguments
--enable-application=browser --enable-optimize --disable-debug

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug  424698, but the real issue here is that the edit action listeners don't fire if the input doesn't have focus.... and in this case it seems not to.  Or something.  Not only does the value disappear, but undo/redo is not available, and that part is NOT a regression.

So the real issue is that editor is not notifying us correctly, and that's what we need to fix.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, oninput not firing on the text control also seems to be part of the same thing and is also not a regression.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So this is weird.  We're certainly adding the event listener to the <input> node (using addEventListenerByIID).  How come it's not getting the events?  Are we screwing up the event target?

Comment 4

[Mats Palmgren (inactive)]

Attachment: wip

I think the change in bug 424698 is correct.  It uncovers an old bug
though - you can see it in Firefox 2 too: the Undo/Redo menu isn't updated.

The problem is that we don't register an observer on the editor for
the new frame (in nsTextInputListener::Focus()). We do get the call,
but for the frame that will be destroyed.
When creating a frame for content that's already focused we don't
get any new focus event (correctly).

This patch fixes the bug, but I'll see if we can do this in
nsTextControlFrame::SetFocus() instead and get rid of the
nsIFocusListener part of nsTextInputListener.
(MXR is not responding so this link is to CVS trunk)
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/forms/nsTextControlFrame.cpp&rev=3.294&root=/cvsroot&mark=342-387#341

Or do you have a better suggestion?

Comment 7

[Mats Palmgren (inactive)]

Attachment: mochitest.diff

Comment 9

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

I tried to move the nsTextInputListener::Focus()/Blur() stuff into
nsTextControlFrame::SetFocus() where I think it really belongs,
but we don't get calls to that method consistently...
and given how fragile focus stuff is in general I don't want
to attempt to fix that at this point.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 331036 [details] [diff] [review]
Patch rev. 1

Looks reasonable.  Please add a mochitest that tests that undo/redo work and oninput is fired (all of which should fail without this patch)?  It'd be great to have a reftest for this bug too, but I'm not sure whether we can run privileged script (needed to simulate typing) in reftest.

Comment 11

[dolphinling]

Any chance of this getting checked in? Thanks.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Checked in changesets 63d939a80c03 and e44b3f9c8824 (forgot to hg add the test file at first).  I added an oninput test to the test file.

Comment 13

[dolphinling]

Awesome.

Apparently undo/redo weren't fixed (completely?), I filed followup bug Bug 448784 on that.