Closed Bug 522787 Opened 11 years ago Closed 11 years ago
Text which was drag-dropped into TEXTAREA/INPUT is lost, after frame reconstruct
403 bytes, text/html
2.88 KB, text/html
1.42 KB, patch
|Details | Diff | Splinter Review|
2.96 KB, text/plain
6.79 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2b2pre) Gecko/20091016 Firefox/3.5.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091016 Minefield/3.7a1pre ID:20091016045204 Text which was drag-dropped into TEXTAREA/INPUT is lost, after detach tab. On the other hand, They are not lost if I enter any key in TEXTAREA/INPUT Reproducible: Always Steps to Reproduce: 1.Start Minefield with new profile 2.Open http://www.mozilla.org/projects/minefield/ 3.Open http://www.mozilla.org/projects/firefox/prerelease.html 4.Select any text on the web page, and drag drop into TEXTAREA/INPUT 5.Detach tab of the page Actual Results: Text which was drag-dropped into TEXTAREA/INPUT is lost, after detach tab. Expected Results: Text which was drag-dropped into TEXTAREA/INPUT should be preserved, after detach tab.
Interesting. This seems like it should be a general issue with form controls....
Status: UNCONFIRMED → NEW
Component: Document Navigation → Layout: Form Controls
Ever confirmed: true
QA Contact: docshell → layout.form-controls
Hmm. Is this an editor bug? I don't see the text input listener's EditAction() called in this case (so in particular we are NOT firing oninput, and we're not flagging ourselves as having a changed value).
Oh, hmm. We don't add the nsTextInputListener unless the control has focus. Dropping doesn't give it focus. Enn, Peter, any thoughts on how this _should_ work? Should we just always have the editor observer set up?
OK, this seems to be a regression between 3.0 and 3.5.
No longer depends on: 113934
Summary: Text which was drag-dropped into TEXTAREA/INPUT is lost, after detach tab. → Text which was drag-dropped into TEXTAREA/INPUT is lost, after frame reconstruct
Regressed between 2008-07-21-02 and 2008-07-22-02. Pushlog range (+2 hours on each end): http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-07-21+00&enddate=2008-07-22+04 Looks like a likely regression from bug 424698. Should we perhaps just be comparing the textframe value to the default value and setting the "value changed" flag based on that? That seems to fail if the user did some editing but then deleted the changes....
Yeah, this seems really bad to me. Boris, can you take it since the regression is yours? Or should peterv have it?
blocking1.9.1: --- → ?
Did you want this bug to block 3.6b1 ? It's appearing in that query right now.
No, blocking the release is fine. Does setting P2 accomplish that?
Priority: -- → P2
(I can reproduce this on a linux mozilla-central nighly build. Platform --> All)
OS: Windows Vista → All
Hardware: x86 → All
Assignee: nobody → matspal
Always having a nsTextInputListener registered seems to work, and it fires oninput. I think we should also fire onchange when necessary (bug 128066). I'll ask for review in a day or two after some testing...
Status: NEW → ASSIGNED
This won't cause any problems when, say, changing the value of an input (either focused or unfocused) from script, will it?
Attachment #408756 - Attachment is obsolete: true
This works as I intended, firing oninput and onchange... but seeing that IE8, Safari and Opera focus the drop target on a successful drop that seems to me as a much nicer behavior, and it's filed as bug 280635 already, then we don't need an explicit onchange either since we'll get it on blur as normal.
This fixes the bug and fires oninput too. (In reply to comment #13) > This won't cause any problems when, say, changing the value of an input (either > focused or unfocused) from script, will it? No problem as a far as I can tell, did you have specific scenario in mind? The patch is cooking on TryServer now...
I tried hard to make a mochitest for this bug, but can't get the drop part work. I was able to create a real DataTransfer for the drag I think. This code is mostly stolen from synthesizeDrop() in SimpleTest/EventUtils.js (which doesn't seem to work either), and content/events/test/test_dragstart.html
Comment on attachment 409070 [details] [diff] [review] Patch rev. 2 ... passed regression tests on TryServer.
Comment on attachment 409070 [details] [diff] [review] Patch rev. 2 I assume this doesn't pick up scripted value changes as editor stuff or anything like that?
Whiteboard: [needs review]
No problem as a far as I can tell, testing various scripted changes with Testcase #3. Do you have specific scenario in mind? (Neil asked the same thing in comment 13.)
> Do you have specific scenario in mind? No, just making sure we don't start firing oninput/onchange for scripted changes. It'd be good to have a mochitest for that if we don't have one already....
Only found content/html/content/test/test_bug388558.html but it tests the opposite, that we do get onchange for user input. I'll write a few simple tests that scripted changes doesn't trigger events...
... that test actually tests some scripted changes too. I'll see if I can add some more cases.
Add more tests to content/html/content/test/test_bug388558.html Now also tests input events, and scripted changes from onfocus/ oninput/onblur.
Comment on attachment 409070 [details] [diff] [review] Patch rev. 2 Olli, maybe you could review this instead of Boris?
Attachment #409070 - Flags: review?(Olli.Pettay)
Mats, did you test undo/redo handling?
I think undo/redo should work just fine, but better to test. I'll test once my build is ready.
Attachment #409070 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 409070 [details] [diff] [review] Patch rev. 2 Would be great to understand the reason for setting observer when focusing, but unfortunately bonsai history seems to end to bug 129909. Perhaps 129909#c4 didn't actually happen :(
> bonsai history seems to end to bug 129909. No, #c4 didn't happen. You can see the pre-rename blame at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla%2Flayout%2Fhtml%2Fforms%2Fsrc%2FAttic%2FnsGfxTextControlFrame2.cpp&rev=&cvsroot=%2Fcvsroot as desired.
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/cce7954f1b99 The tests still need to be landed.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 192 landing]
(In reply to comment #27) > I think undo/redo should work just fine, but better to test. Yes, undo/redo works fine for undoing/redoing the dropped text. After clicking the button (recreating the frame) the edit history is lost though (for any edit, not just drop). That seems like a bug to me, but expected since the editor is owned by the frame. (bug 343812 should fix that)
You need to log in before you can comment on or make changes to this bug.