Closed
Bug 1386484
Opened 7 years ago
Closed 7 years ago
Make SetTextTransaction less expensive to create
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla57
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
No description provided.
|
Assignee | |
Comment 1•7 years ago
|
||
Attachment #8892718 -
Flags: review?(masayuki)
|
|
Assignee | |
Comment 2•7 years ago
|
||
Attachment #8892719 -
Flags: review?(masayuki)
|
||
Updated•7 years ago
|
Assignee: nobody → ehsan
|
||
Comment 3•7 years ago
|
||
Comment on attachment 8892718 [details] [diff] [review]
Part 1: Inline SetTextTransaction::SetTextTransaction()
I wonder, why don't we move the code in SetTextTransaction::DoTransaction() to EditorBase::SetTextImpl() or make it a new method of EditorBase? Then, we don't need to create the instance.
Flags: needinfo?(ehsan)
|
|
Assignee | |
Comment 4•7 years ago
|
||
You're right, this class has really lost its purpose to exist now!
Flags: needinfo?(ehsan)
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8892718 -
Attachment is obsolete: true
Attachment #8892718 -
Flags: review?(masayuki)
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8892719 -
Attachment is obsolete: true
Attachment #8892719 -
Flags: review?(masayuki)
|
|
Assignee | |
Comment 5•7 years ago
|
||
Besides some unnecessary copying and malloc overhead that this removes, it
also removes a call to dom::Text::GetData() which we used to only make to
get the old length of the text node before modifying it. Turns out that
this old length was already obtained once in SetTextImpl().
Attachment #8893144 -
Flags: review?(masayuki)
|
|
||
Comment 6•7 years ago
|
||
Comment on attachment 8893144 [details] [diff] [review]
Remove the SetTextTransaction class and embed its functionality into EditorBase::SetTextImpl()
Excellent!
Attachment #8893144 -
Flags: review?(masayuki) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/baeb2b439c08
Remove the SetTextTransaction class and embed its functionality into EditorBase::SetTextImpl(); r=masayuki
|
||
Comment 8•7 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•