Closed Bug 489851 Opened 11 years ago Closed 11 years ago

remove TransactionFactory

Categories

(Core :: DOM: Editor, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: dbaron, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Editor has a transaction factory that doesn't seem to serve any useful purpose.  I noticed this while working on bug 488799.

This patch removes it.  It makes the style of the functions that used to call it a little bit more uniform (i.e., converts some to early-return rather than putting the whole function inside an if).

It also removes almost all of the transaction CIDs.  Before this patch, three transactions had CIDs that were also used in QueryInterface:  EditAggregateTxn, IMETextTxn, and InsertTextTxn.  The first one's presence in QueryInterface was unused (un-queried-for).  So this removes all transaction CIDs except for those on IMETextTxn and InsertTextTxn, and removes a slow layer of indirection through TransactionFactory.
Attachment #374318 - Flags: superreview?(peterv)
Attachment #374318 - Flags: review?(peterv)
Attachment #374318 - Flags: superreview?(peterv)
Attachment #374318 - Flags: superreview+
Attachment #374318 - Flags: review?(peterv)
Attachment #374318 - Flags: review+
Comment on attachment 374318 [details] [diff] [review]
patch

>diff --git a/editor/libeditor/base/nsEditor.cpp b/editor/libeditor/base/nsEditor.cpp

> nsEditor::CreateTxnForRemoveAttribute(nsIDOMElement *aElement, 

>+  nsAutoString value;
>+  return (*aTxn)->Init(this, aElement, aAttribute, value, PR_TRUE);

EmptyString()?

> nsEditor::CreateTxnForIMEText(const nsAString& aStringToInsert,

>+  return (*aTxn)->Init(mIMETextNode,mIMETextOffset,mIMEBufferLength,mIMETextRangeList,aStringToInsert,mSelConWeak);

Add some spaces and wrap.
http://hg.mozilla.org/mozilla-central/rev/cec9d9f355e8

(Reordering the patch with bug 488799 made the diff churn a bit.)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Priority: -- → P4
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.