Incorrect height measured when "overflow" style is changed in textarea before bug 1787088, because of editor re-initialization
Categories
(Core :: DOM: Editor, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr91 | --- | unaffected |
firefox-esr102 | --- | unaffected |
firefox104 | --- | wontfix |
firefox105 | --- | wontfix |
firefox106 | --- | fix-optional |
People
(Reporter: dl1644, Unassigned)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/html
|
Details |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36
Steps to reproduce:
- Go to https://codepen.io/dl1644/pen/oNqOOaL?editors=1111
- Type into the left most textarea until the text's height exceeds the textarea's height.
- Type into the right most textarea until the textarea's begins to grow.
Actual results:
Note that the left most textarea's height doesn't grow to match the text's height but the right most textarea's height does.
Expected results:
Both textareas should grow to match the text's height as you type into them. Before Firefox 104, we were able to change a textarea's overflow style to "hidden" to remove any vertical scrollbars that may be present in the textarea and thus measure the true height of the textarea's text. We would then set the textarea's height to the measured height, resulting in a textarea that would grow/shrink to fit the amount of text within it.
After Firefox 104, the above no longer works as demonstrated by the left most textarea. Substituting the "overflow: hidden" approach with "scrollbar-width: none" doesn't work either. Note that getting rid of the "target.style.overflow = prevOverflow;" line fixes the height measurement but we want to preserve the previous "overflow" setting. Note that the above codepen works in Chrome and Safari.
Comment 1•2 years ago
|
||
Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3ea7d91368d10e6322bb72f5c1c59c624cb3cd11&tochange=d3176de5dffaa8527ae417ad2e75c21d02b87d62
Comment 2•2 years ago
|
||
Thanks for the report, will look.
It seems like something is triggering a style change while processing the previous style change, which is obviously unsound, and before my patch was getting wallpapered by an extra style update. Workaround without any negative side effects should be to call target.scrollHeight
or anything else that updates layout before updating the computation. e.g:
overflowTextArea.addEventListener("input", (e) => {
let {target} = e;
let prevOverflow = target.style.overflow;
target.style.overflow = 'hidden';
target.style.height = 'auto';
target.scrollHeight; // Inserted this line.
target.style.height = `${target.scrollHeight + (target.offsetHeight - target.clientHeight)}px`;
target.style.overflow = prevOverflow;
});
Comment 3•2 years ago
|
||
Comment 4•2 years ago
|
||
Bug 1787088 adds an optimization that avoids this bug, but there's more to do potentially, since I haven't found the root cause yet.
Comment 5•2 years ago
|
||
Ok, so the root cause is that the overflow change causes us to re-initialize the editor because of the re-frame, and thus we end up with a restyle where the editor content is not there yet. The double ProcessPendingRestyles
wallpapered over this.
Masayuki, I know it is probably a can of worms, but how hard would it be to make sure that the editor anonymous content is ready when we actually create the anonymous content (so in nsTextControlFrame::CreateAnonymousContent
)?
This "need a script runner to go out of scope to create a sane layout" is incompatible with container queries.
Updated•2 years ago
|
Comment 6•2 years ago
|
||
Ah, for reference, editor initialization stack:
(rr) bt
#0 nsCOMPtr<nsINode>::assign_assuming_AddRef(nsINode*) (this=0x7fa2d4e2d0a0, aNewPtr=0x7fa2d1d0c550) at /home/emilio/src/moz/gecko-2/obj-debug/dist/include/nsCOMPtr.h:423
#1 0x00007fa2e919234e in nsCOMPtr<nsINode>::assign_with_AddRef(nsISupports*) (this=0x7fa2d4e2d0a0, aRawPtr=0x7fa2d1d0c550) at /home/emilio/src/moz/gecko-2/obj-debug/dist/include/nsCOMPtr.h:1179
#2 nsCOMPtr<nsINode>::operator=(nsINode*) (this=0x7fa2d4e2d0a0, aRhs=0x7fa2d1d0c550) at /home/emilio/src/moz/gecko-2/obj-debug/dist/include/nsCOMPtr.h:690
#3 mozilla::dom::Document::SetServoRestyleRoot(nsINode*, unsigned int) (this=0x7fa2d4e2c700, this@entry=0x1, aRoot=aRoot@entry=0x7fa2d1d0c550, aDirtyBits=4096, aDirtyBits@entry=2271844120)
at /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/dom/DocumentInlines.h:39
#4 0x00007fa2e916840d in mozilla::dom::NoteDirtyElement(mozilla::dom::Element*, unsigned int) (aElement=0x7fa2d1d0c550, aBits=4107932576, aBits@entry=4096) at /home/emilio/src/moz/gecko-2/dom/base/Element.cpp:4574
#5 0x00007fa2e9168802 in mozilla::dom::Element::NoteDescendantsNeedFramesForServo() (this=0x7fa2d4e2d0a0) at /home/emilio/src/moz/gecko-2/dom/base/Element.cpp:4670
#6 0x00007fa2eb7b9d58 in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsCSSFrameConstructor::InsertionKind) (this=0x7fa2f4da8e80, aFirstNewContent=0x7fa2d1d06800, aInsertionKind=nsCSSFrameConstructor::InsertionKind::Async)
at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:6788
#7 0x00007fa2eb773606 in mozilla::PresShell::ContentAppended(nsIContent*) (this=<optimized out>, aFirstNewContent=0x7fa2d1d06800) at /home/emilio/src/moz/gecko-2/layout/base/PresShell.cpp:4551
#8 0x00007fa2e91bcd29 in mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*)::$_10::operator()(mozilla::PresShell*) const (aPresShell=0x7fa2d4e2d0a0, this=<optimized out>)
at /home/emilio/src/moz/gecko-2/dom/base/MutationObservers.cpp:162
#9 Notify<(IsRemoval)0, (ShouldAssert)1, mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*)::$_10, mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*)::$_11>(nsINode*, mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*)::$_10&, mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*)::$_11&)
(aNode=<optimized out>, aNotifyPresShell=<optimized out>, aNotifyObserver=<optimized out>) at /home/emilio/src/moz/gecko-2/dom/base/MutationObservers.cpp:97
#10 mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*) (aContainer=<optimized out>, aFirstNewContent=0x7fa2d1d06800) at /home/emilio/src/moz/gecko-2/dom/base/MutationObservers.cpp:163
#11 0x00007fa2e9282100 in nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool, mozilla::ErrorResult&) (this=0x7fa2d1d0c550, aKid=0x7fa2d1d06800, aBeforeThis=0x0, aNotify=false, aRv=...)
at /home/emilio/src/moz/gecko-2/dom/base/nsINode.cpp:1601
#12 0x00007fa2eb57aea7 in mozilla::EditorBase::EnsureEmptyTextFirstChild() (this=this@entry=0x7fa2f4df1e00) at /home/emilio/src/moz/gecko-2/editor/libeditor/EditorBase.cpp:371
#13 0x00007fa2eb57b1af in mozilla::EditorBase::InitEditorContentAndSelection() (this=0x7fa2f4df1e00) at /home/emilio/src/moz/gecko-2/editor/libeditor/EditorBase.cpp:383
#14 0x00007fa2eb66d62d in mozilla::TextEditor::Init(mozilla::dom::Document&, mozilla::dom::Element&, nsISelectionController&, unsigned int, mozilla::UniquePtr<mozilla::PasswordMaskData, mozilla::DefaultDelete<mozilla::PasswordMaskData> >&&) (this=0x7fa2f4df1e00, aDocument=..., aAnonymousDivElement=..., aSelectionController=..., aFlags=<optimized out>, aPasswordMaskData=<optimized out>) at /home/emilio/src/moz/gecko-2/editor/libeditor/TextEditor.cpp:139
#15 0x00007fa2ea609e0e in mozilla::TextControlState::PrepareEditor(nsTSubstring<char16_t> const*) (this=0x7fa2d3caea00, aValue=aValue@entry=0x7fa2d3cc5498) at /home/emilio/src/moz/gecko-2/dom/html/TextControlState.cpp:1846
#16 0x00007fa2ea624bf1 in mozilla::PrepareEditorEvent::Run() (this=<optimized out>) at /home/emilio/src/moz/gecko-2/dom/html/TextControlState.cpp:1608
#17 0x00007fa2e901fae7 in nsContentUtils::RemoveScriptBlocker() () at /home/emilio/src/moz/gecko-2/dom/base/nsContentUtils.cpp:5880
#18 0x00007fa2eb771ecf in nsAutoScriptBlocker::~nsAutoScriptBlocker() (this=<optimized out>) at /home/emilio/src/moz/gecko-2/dom/base/nsContentUtils.h:3565
#19 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) (this=0x7fa2d3c83000, aFlush=...) at /home/emilio/src/moz/gecko-2/layout/base/PresShell.cpp:4375
#20 0x00007fa2e9142c5c in mozilla::PresShell::FlushPendingNotifications
Bug 1787088 makes this possibly less pressing. For some reason I thought it'd be trivial to repro switching display rather than overflow, but I couldn't create a test-case... But it's way too late today, so maybe I messed it up.
Comment 7•2 years ago
|
||
If it's fine to set the value at calling TextEditor::Init
for layout (i.e., inserting the text node with default/current value), I think that the caller of EnsureEmptyTextFirstChild()
should be changed from EditorBase::InitEditorContentAndSelection
to TextEditor::Init
, and make TextEditor::Init
take the initial value. Then, I guess it'll work for the reported case.
However, if the editor is for <textarea>
and it ends with empty line, a padding <br>
for making the empty last line visible.
https://searchfox.org/mozilla-central/rev/30ac44262374be08ccd4262ee36f0716152868d3/editor/libeditor/EditorBase.cpp#408
So I think that the approach does not work in the case. I guess that we need to put it off to append the anonymous <div>
or make TextControlState
reuse TextEditor
and the anonymous subtree if possible. (E.g., putting off to clean up them in the next event loop.) So I think it requires a big change.
Comment 8•2 years ago
|
||
Ah, but I don't know when nsTextControlFrame::CreateAnonymousContent
is called. I assume that it's called before the call of TextEditor::Init
.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
Thanks for the report, will look.
It seems like something is triggering a style change while processing the previous style change, which is obviously unsound, and before my patch was getting wallpapered by an extra style update. Workaround without any negative side effects should be to call
target.scrollHeight
or anything else that updates layout before updating the computation. e.g:overflowTextArea.addEventListener("input", (e) => { let {target} = e; let prevOverflow = target.style.overflow; target.style.overflow = 'hidden'; target.style.height = 'auto'; target.scrollHeight; // Inserted this line. target.style.height = `${target.scrollHeight + (target.offsetHeight - target.clientHeight)}px`; target.style.overflow = prevOverflow; });
Thanks for the workaround!
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•