Closed Bug 1386471 Opened 3 years ago Closed 3 years ago

Don't remove all ranges before calling SetText()

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Attachment #8892699 - Flags: review?(masayuki)
Assignee: nobody → ehsan
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?
Attachment #8892699 - Flags: review?(masayuki) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05400533498b
Don't remove all ranges before calling SetText(); r=masayuki
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...
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(ehsan)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.