Closed Bug 446663 Opened 16 years ago Closed 16 years ago

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

Categories

(Core :: Layout: Form Controls, defect)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: aguertin+bugzilla, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug, )

Details

(Keywords: regression, testcase)

Attachments

(2 files, 1 obsolete file)

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
Version: unspecified → Trunk
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.
Blocks: 424698
Status: UNCONFIRMED → NEW
Ever confirmed: true
Oh, oninput not firing on the text control also seems to be part of the same thing and is also not a regression.
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?
Flags: wanted1.9.1?
Attached patch wip (obsolete) (deleted) — Splinter Review
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?
Assignee: nobody → mats.palmgren
Status: NEW → ASSIGNED
Attached patch mochitest.diff (deleted) — Splinter Review
Attached patch Patch rev. 1 (deleted) — Splinter Review
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.
Attachment #330921 - Attachment is obsolete: true
Attachment #331036 - Flags: superreview?(bzbarsky)
Attachment #331036 - Flags: review?(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.
Attachment #331036 - Flags: superreview?(bzbarsky)
Attachment #331036 - Flags: superreview+
Attachment #331036 - Flags: review?(bzbarsky)
Attachment #331036 - Flags: review+
Any chance of this getting checked in? Thanks.
Checked in changesets 63d939a80c03 and e44b3f9c8824 (forgot to hg add the test file at first). I added an oninput test to the test file.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: wanted1.9.1? → in-testsuite+
Resolution: --- → FIXED
Blocks: 448784
Awesome. Apparently undo/redo weren't fixed (completely?), I filed followup bug Bug 448784 on that.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: