1386471 - Don't remove all ranges before calling SetText()

Status: RESOLVED WONTFIX

(Core :: DOM: Editor, enhancement)

Description

[(no longer active)]

As I mentioned in bug 1376856 comment 11, this is unnecessary due to the selection management in AfterEdit() and this shows up heavily in profiles.

Comment 1

[(no longer active)]

Attachment: Don't remove all ranges before calling SetText()

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8892699 [details] [diff] [review]
Don't remove all ranges before calling SetText()

>diff --git a/dom/html/nsTextEditorState.cpp b/dom/html/nsTextEditorState.cpp
>@@ -2668,21 +2668,16 @@ nsTextEditorState::SetValue(const nsAString& aValue, const nsAString* aOldValue,
> 
>             if (insertValue.IsEmpty()) {
>               textEditor->DeleteSelection(nsIEditor::eNone, nsIEditor::eStrip);
>             } else {
>               textEditor->InsertText(insertValue);
>             }
>           } else {
>             AutoDisableUndo disableUndo(textEditor);
>-            if (domSel) {
>-              // Since we don't use undo transaction, we don't need to store
>-              // selection state.  SetText will set selection to tail.
>-              domSel->RemoveAllRanges();
>-            }
> 
>             textEditor->SetText(newValue);

nit: could you remove the empty line too?

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05400533498b
Don't remove all ranges before calling SetText(); r=masayuki

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for failures in browser_contentSearchUI.js like https://treeherder.mozilla.org/logviewer.html#?job_id=120553056&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e16fd389e84

Comment 5

[(no longer active)]

Huh, OK this code has clearly some stuff depend on it.  While that is somewhat troubling, I doubt it is worth investigating right now.  Let's WONTFIX...