Closed
Bug 500905
Opened 16 years ago
Closed 16 years ago
scroll and caret position of a textarea resets during reflow
Categories
(Core :: Layout: Form Controls, defect)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
People
(Reporter: syskin2, Assigned: bzbarsky)
References
Details
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090626 Firefox/3.6a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090626 Minefield/3.6a1pre
See attached testcase. I'm not sure if it's the reflow that's causing it, but something is.
Reproducible: Always
Steps to Reproduce:
1. Open attached testcase
2. Type something into text area
3. Use left arrow to move the caret back and attempt to type something in the middle
Actual Results:
When the "preview" box under text area updates (on every keypress), textarea scrolling is reset to zero and caret is put at the end, making editing impossible.
Expected Results:
Preview has no effect.
This is a reduced testcase from a big forum, where this kind of logic is used to preview the post as it's being typed or edited.
This regressed ~2-4 days ago. I will attempt to get a detailed regression rage.
Reporter | ||
Comment 1•16 years ago
|
||
Possibly same as this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=496649
I can reproduce this error using Minefield 1.9.2a1pre build 20090626, but the error doesn't seem to occur when using Minefield 1.9.2a1pre build 20090611.
So the bug was introduced after build 20090611.
Could be related to the patch that landed in the 20090618 Trunk:
http://www.squarefree.com/burningedge/2009/06/18/2009-06-18-trunk-builds/
https://bugzilla.mozilla.org/show_bug.cgi?id=178324
Comment 4•16 years ago
|
||
Very likely related to bug 496649, as the key thing in this testcase is the <p> tags in the innerHTML of the span. Except this case is a recent regression, whereas bug 496649 goes back at least as far as Firefox 2.
Regression range for this bug http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c575412d976a&tochange=5fe89f2c22f0
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows Vista → All
Hardware: x86 → All
Version: unspecified → Trunk
Assignee | ||
Comment 5•16 years ago
|
||
Probably a regression from bug 495385. I bet we're reframing on every char.
Bug 500556 might help.
Assignee | ||
Comment 6•16 years ago
|
||
On the other hand, this text should be white-space:pre or some such and hence not get the "is it whitespace?" bits. Why is it getting them at all, assuming that's what happens?
Here's the stack for the reconstruction of the textarea:
0 nsBoxFrame::Init (this=0x172b8dc, aContent=0x1026aa60, aParent=0x173cb54, aPrevInFlow=0x0) at /Users/roc/mozilla-checkin/layout/xul/base/src/nsBoxFrame.cpp:203
#1 0x01eb74c9 in nsCSSFrameConstructor::InitAndRestoreFrame (this=0x10259300, aState=@0xbfffbd98, aContent=0x1026aa60, aParentFrame=0x173cb54, aPrevInFlow=0x0, aNewFrame=0x172b8dc, aAllowCounters=1) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:4704
#2 0x01ebf18c in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x10259300, aItem=@0xd1f4b00, aState=@0xbfffbd98, aParentFrame=0x173cb54, aFrameItems=@0xbfffb2e0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:3927
#3 0x01ebf912 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x10259300, aState=@0xbfffbd98, aIter=@0xbfffaff4, aParentFrame=0x173cb54, aFrameItems=@0xbfffb2e0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:5577
#4 0x01ebfaa3 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x10259300, aState=@0xbfffbd98, aItems=@0xbfffb1a8, aParentFrame=0x173cb54, aFrameItems=@0xbfffb2e0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9458
#5 0x01ec0b6f in nsCSSFrameConstructor::ProcessChildren (this=0x10259300, aState=@0xbfffbd98, aContent=0x1026aa20, aStyleContext=0x172b314, aFrame=0x173cb54, aCanHaveGeneratedContent=1, aFrameItems=@0xbfffb2e0, aAllowBlockStyles=1) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9571
#6 0x01ec4171 in nsCSSFrameConstructor::ConstructTableCell (this=0x10259300, aState=@0xbfffbd98, aItem=@0xd19aa70, aParentFrame=0x172b290, aDisplay=0x172b384, aFrameItems=@0xbfffb764, aNewFrame=0xbfffb3b4) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:2338
#7 0x01ebef75 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x10259300, aItem=@0xd19aa70, aState=@0xbfffbd98, aParentFrame=0x172b290, aFrameItems=@0xbfffb764) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:3888
#8 0x01ebf912 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x10259300, aState=@0xbfffbd98, aIter=@0xbfffb494, aParentFrame=0x172b290, aFrameItems=@0xbfffb764) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:5577
#9 0x01ebfaa3 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x10259300, aState=@0xbfffbd98, aItems=@0xbfffb648, aParentFrame=0x172b290, aFrameItems=@0xbfffb764) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9458
#10 0x01ec0b6f in nsCSSFrameConstructor::ProcessChildren (this=0x10259300, aState=@0xbfffbd98, aContent=0x1026a8e0, aStyleContext=0x172af50, aFrame=0x172b290, aCanHaveGeneratedContent=1, aFrameItems=@0xbfffb764, aAllowBlockStyles=0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9571
#11 0x01ec43bd in nsCSSFrameConstructor::ConstructTableRow (this=0x10259300, aState=@0xbfffbd98, aItem=@0x9225dc0, aParentFrame=0x172b0d8, aDisplay=0x172b188, aFrameItems=@0xbfffbc28, aNewFrame=0xbfffb824) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:2196
#12 0x01ebef75 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x10259300, aItem=@0x9225dc0, aState=@0xbfffbd98, aParentFrame=0x172b0d8, aFrameItems=@0xbfffbc28) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:3888
#13 0x01ebf912 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x10259300, aState=@0xbfffbd98, aIter=@0xbfffb904, aParentFrame=0x172b0d8, aFrameItems=@0xbfffbc28) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:5577
#14 0x01ebfaa3 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x10259300, aState=@0xbfffbd98, aItems=@0xbfffbab8, aParentFrame=0x172b0d8, aFrameItems=@0xbfffbc28) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9458
#15 0x01ec0b6f in nsCSSFrameConstructor::ProcessChildren (this=0x10259300, aState=@0xbfffbd98, aContent=0x1026a8a0, aStyleContext=0x172afe8, aFrame=0x172b0d8, aCanHaveGeneratedContent=1, aFrameItems=@0xbfffbc28, aAllowBlockStyles=0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9571
#16 0x01ebf426 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x10259300, aItem=@0xd644f90, aState=@0xbfffbd98, aParentFrame=0x172aeb0, aFrameItems=@0xbfffbe20) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:3972
#17 0x01ebf912 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x10259300, aState=@0xbfffbd98, aIter=@0xbfffbd14, aParentFrame=0x172aeb0, aFrameItems=@0xbfffbe20) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:5577
#18 0x01ebfaa3 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x10259300, aState=@0xbfffbd98, aItems=@0xbfffbde0, aParentFrame=0x172aeb0, aFrameItems=@0xbfffbe20) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9458
#19 0x01ec6c48 in nsCSSFrameConstructor::ContentInserted (this=0x10259300, aContainer=0x1026a800, aChild=0x1026a8a0, aIndexInContainer=1, aFrameState=0x1026a150) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:6775
#20 0x01ec80f5 in nsCSSFrameConstructor::RecreateFramesForContent (this=0x10259300, aContent=0x1026a8a0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9080
#21 0x01ec860e in nsCSSFrameConstructor::WipeContainingBlock (this=0x10259300, aState=@0xbfffbfe8, aContainingBlock=0x172aa90, aFrame=0x172b0d8, aItems=@0xbfffc030, aIsAppend=0, aPrevSibling=0x172b290) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:11118
#22 0x01ec6bbe in nsCSSFrameConstructor::ContentInserted (this=0x10259300, aContainer=0x1026a8a0, aChild=0x1026ac70, aIndexInContainer=2, aFrameState=0x1026a150) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:6763
#23 0x01ec80f5 in nsCSSFrameConstructor::RecreateFramesForContent (this=0x10259300, aContent=0x1026ac70) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9080
#24 0x01ec860e in nsCSSFrameConstructor::WipeContainingBlock (this=0x10259300, aState=@0xbfffc238, aContainingBlock=0x172aa90, aFrame=0x1725210, aItems=@0xbfffc280, aIsAppend=0, aPrevSibling=0x17253dc) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:11118
#25 0x01ec6bbe in nsCSSFrameConstructor::ContentInserted (this=0x10259300, aContainer=0x1026ac70, aChild=0x1026ad80, aIndexInContainer=3, aFrameState=0x1026a150) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:6763
#26 0x01ec80f5 in nsCSSFrameConstructor::RecreateFramesForContent (this=0x10259300, aContent=0x1026ad80) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9080
#27 0x01ec82fc in nsCSSFrameConstructor::ReframeContainingBlock (this=0x10259300, aFrame=0x173cc0c) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:11272
#28 0x01ec8d98 in nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval (this=0x10259300, aFrame=0x172cb30, aResult=0xbfffc4f0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9007
#29 0x01ec76f6 in nsCSSFrameConstructor::ContentRemoved (this=0x10259300, aContainer=0x1026ae00, aChild=0xd6bbe50, aIndexInContainer=0, aFlags=nsCSSFrameConstructor::REMOVE_CONTENT, aDidReconstruct=0xbfffc598) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:7196
#30 0x01f2eee4 in PresShell::ContentRemoved (this=0x170aa00, aDocument=0x1447800, aContainer=0x1026ae00, aChild=0xd6bbe50, aIndexInContainer=0) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:5052
#31 0x0221b186 in nsNodeUtils::ContentRemoved (aContainer=0x1026ae00, aChild=0xd6bbe50, aIndexInContainer=0) at /Users/roc/mozilla-checkin/content/base/src/nsNodeUtils.cpp:167
#32 0x022054e9 in nsGenericElement::doRemoveChildAt (aIndex=0, aNotify=1, aKid=0xd6bbe50, aParent=0x1026ae00, aDocument=0x1447800, aChildArray=@0x1026ae1c) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3326
#33 0x02205635 in nsGenericElement::RemoveChildAt (this=0x1026ae00, aIndex=0, aNotify=1) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3256
#34 0x0219b7bb in nsContentUtils::SetNodeTextContent (aContent=0x1026ae00, aValue=@0x5c09c0, aTryReuse=0) at /Users/roc/mozilla-checkin/content/base/src/nsContentUtils.cpp:3781
#35 0x022ba7bc in nsGenericHTMLElement::SetInnerHTML (this=0x1026ae00, aInnerHTML=@0xbfffc894) at /Users/roc/mozilla-checkin/content/html/content/src/nsGenericHTMLElement.cpp:688
We're setting the innerHTML of the <span> to a <p>, so that causes reframing of the containing block (the table cell), which triggers reframing of the row (don't know why), which triggers reframing of the table body which is element for the table itself.
Several problems here:
-- the caret position doesn't survive reframing of the textarea
-- we should be able to reframe just the children of the table cell instead of the table cell itself
-- I don't see why reframing the cell should reframe the row
-- I don't see how this is connected to bug 495385
Assignee | ||
Comment 8•16 years ago
|
||
> we should be able to reframe just the children of the table cell instead of
> the table cell itself
dbaron and I have been thinking about doing this, yeah. We should try to just do it.
> -- I don't see why reframing the cell should reframe the row
> -- I don't see how this is connected to bug 495385
These are almost certainly related. That said, I would have expected bug 500467 to fix this. At least for the "remove the cell" case. I suppose it might still be a problem for the "insert the cell" case. Maybe we need a similar parent frame type check there?
Assignee | ||
Comment 9•16 years ago
|
||
This is a "remove for reconstruct", so I bet that's exactly the issue. I should be able to put a patch together for this tomorrow.
Comment 10•16 years ago
|
||
Should we maybe make testarea implement nsIStatefulFrame so that in any other cases of reconstruction we save the scroll/caret position?
Yes, that would be cool.
Assignee | ||
Comment 12•16 years ago
|
||
A few things going on here:
1) Modified our existing crashtest to test inserts, not just removes. This
immediately showed the O(N^2) behavior due to reframing on every insert.
2) Fixed WipeContainingBlock to be smarter about trailing whitespace so as not
to actually trigger reframes here. That got me down to about 30% slower
than before bug 495385 landed.
3) Changed ContentAppended and ContentInserted to check parent type before
trying to even create the items for those whitespace nodes. That got back
the 30% loss, so now we're as fast as nightlies from May or November on
that testcase.
I did check that the patch also fixes the testcase in this bug. That's not tested by the crashtest, since that could also pass if we did lazy reframes...
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #385912 -
Flags: superreview?(roc)
Attachment #385912 -
Flags: review?(roc)
Attachment #385912 -
Flags: superreview?(roc)
Attachment #385912 -
Flags: superreview+
Attachment #385912 -
Flags: review?(roc)
Attachment #385912 -
Flags: review+
Assignee | ||
Comment 13•16 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/8a901fb8e927
Timothy, that sounds like a good idea; file a bug on that?
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment 14•16 years ago
|
||
(In reply to comment #13)
> Timothy, that sounds like a good idea; file a bug on that?
I think we may as well use bug 496649 as the bug for that.
You need to log in
before you can comment on or make changes to this bug.
Description
•