Bug 1425412 - part 4: Create CompositionTransaction::Create() and remove EditorBase::CreateTxnForComposition() r?m_kato
EditorBase::CreateTxnForComposition() just hides what it does. For its caller,
creating a factory method, CompositionTransaction::Create(), is clearer what
it does.
Additionally, capsuling the creation in CompositionTransaction class can make
the text node information in TextComposition updated automatically. So, now,
it might be better to update them in DoTransaction() because there is a lag
between creating the transaction and calling DoTransaction(). However, this
patch doesn't want to change any existing behavior. So, this doesn't fix this
issue.
MozReview-Commit-ID: K8ci7ytwh1u
--- a/dom/events/TextComposition.h
+++ b/dom/events/TextComposition.h
@@ -141,21 +141,22 @@ public:
*/
uint32_t XPOffsetInTextNode() const
{
return mCompositionStartOffsetInTextNode;
}
/**
* The length of composition string in the text node. If composition string
- * hasn't been inserted in any text node yet, this returns UINT32_MAX.
+ * hasn't been inserted in any text node yet, this returns 0.
*/
uint32_t XPLengthInTextNode() const
{
- return mCompositionLengthInTextNode;
+ return mCompositionLengthInTextNode == UINT32_MAX ?
+ 0 : mCompositionLengthInTextNode;
}
/**
* The end offset of composition string in the text node. If composition
* string hasn't been inserted in any text node yet, this returns UINT32_MAX.
*/
uint32_t XPEndOffsetInTextNode() const
{
@@ -231,57 +232,44 @@ public:
private:
RefPtr<TextComposition> mComposition;
CompositionChangeEventHandlingMarker();
CompositionChangeEventHandlingMarker(
const CompositionChangeEventHandlingMarker& aOther);
};
/**
- * WillCreateCompositionTransaction() is called by the focused editor
- * immediately before creating CompositionTransaction.
+ * OnCreateCompositionTransaction() is called by
+ * CompositionTransaction::Create() immediately after creating
+ * new CompositionTransaction instance.
*
- * @param aTextNode The text node which includes composition string.
- * @param aOffset The offset of composition string in aTextNode.
+ * @param aStringToInsert The string to insert the text node actually.
+ * This may be different from the data of
+ * dispatching composition event because it may
+ * be replaced with different character for
+ * passwords, or truncated due to maxlength.
+ * @param aTextNode The text node which includes composition string.
+ * @param aOffset The offset of composition string in aTextNode.
*/
- void WillCreateCompositionTransaction(Text* aTextNode,
- uint32_t aOffset)
+ void OnCreateCompositionTransaction(const nsAString& aStringToInsert,
+ Text* aTextNode,
+ uint32_t aOffset)
{
if (!mContainerTextNode) {
mContainerTextNode = aTextNode;
mCompositionStartOffsetInTextNode = aOffset;
NS_WARNING_ASSERTION(mCompositionStartOffsetInTextNode != UINT32_MAX,
"The text node is really too long.");
}
#ifdef DEBUG
else {
- NS_WARNING_ASSERTION(aTextNode == mContainerTextNode,
- "The editor tries to insert composition string into different node");
- NS_WARNING_ASSERTION(aOffset == mCompositionStartOffsetInTextNode,
- "The editor tries to insert composition string into different offset");
+ MOZ_ASSERT(aTextNode == mContainerTextNode);
+ MOZ_ASSERT(aOffset == mCompositionStartOffsetInTextNode);
}
#endif // #ifdef DEBUG
- if (mCompositionLengthInTextNode == UINT32_MAX) {
- mCompositionLengthInTextNode = 0;
- }
- }
-
- /**
- * DidCreateCompositionTransaction() is called by the focused editor
- * immediately after creating CompositionTransaction.
- *
- * @param aStringToInsert The string to insert the text node actually.
- * This may be different from the data of
- * dispatching composition event because it may
- * be replaced with different character for
- * passwords, or truncated due to maxlength.
- */
- void DidCreateCompositionTransaction(const nsAString& aStringToInsert)
- {
- MOZ_ASSERT(mCompositionStartOffsetInTextNode != UINT32_MAX);
mCompositionLengthInTextNode = aStringToInsert.Length();
NS_WARNING_ASSERTION(mCompositionLengthInTextNode != UINT32_MAX,
"The string to insert is really too long.");
}
/**
* OnTextNodeRemoved() is called when focused editor is reframed and
* mContainerTextNode may be (or have been) replaced with different text
--- a/editor/libeditor/CompositionTransaction.cpp
+++ b/editor/libeditor/CompositionTransaction.cpp
@@ -16,28 +16,62 @@
#include "nsIPresShell.h" // nsISelectionController constants
#include "nsRange.h" // local var
#include "nsQueryObject.h" // for do_QueryObject
namespace mozilla {
using namespace dom;
+// static
+already_AddRefed<CompositionTransaction>
+CompositionTransaction::Create(EditorBase& aEditorBase,
+ const nsAString& aStringToInsert,
+ Text& aTextNode,
+ uint32_t aOffset)
+{
+ TextComposition* composition = aEditorBase.GetComposition();
+ MOZ_RELEASE_ASSERT(composition);
+ // XXX Actually, we get different text node and offset from editor in some
+ // cases. If composition stores text node, we should use it and offset
+ // in it.
+ Text* textNode = composition->GetContainerTextNode();
+ uint32_t offset;
+ if (textNode) {
+ offset = composition->XPOffsetInTextNode();
+ NS_WARNING_ASSERTION(&aTextNode == composition->GetContainerTextNode(),
+ "The editor tries to insert composition string into different node");
+ NS_WARNING_ASSERTION(aOffset == composition->XPOffsetInTextNode(),
+ "The editor tries to insert composition string into different offset");
+ } else {
+ textNode = &aTextNode;
+ offset = aOffset;
+ }
+ RefPtr<CompositionTransaction> transaction =
+ new CompositionTransaction(aEditorBase, aStringToInsert,
+ *textNode, offset);
+ // XXX Now, it might be better to modify the text node information of
+ // the TextComposition instance in DoTransaction() because updating
+ // the information before changing actual DOM tree is pretty odd.
+ composition->OnCreateCompositionTransaction(aStringToInsert,
+ textNode, offset);
+ return transaction.forget();
+}
+
CompositionTransaction::CompositionTransaction(
EditorBase& aEditorBase,
const nsAString& aStringToInsert,
- const TextComposition& aTextComposition,
- RangeUpdater* aRangeUpdater)
- : mTextNode(aTextComposition.GetContainerTextNode())
- , mOffset(aTextComposition.XPOffsetInTextNode())
- , mReplaceLength(aTextComposition.XPLengthInTextNode())
- , mRanges(aTextComposition.GetRanges())
+ Text& aTextNode,
+ uint32_t aOffset)
+ : mTextNode(&aTextNode)
+ , mOffset(aOffset)
+ , mReplaceLength(aEditorBase.GetComposition()->XPLengthInTextNode())
+ , mRanges(aEditorBase.GetComposition()->GetRanges())
, mStringToInsert(aStringToInsert)
, mEditorBase(&aEditorBase)
- , mRangeUpdater(aRangeUpdater)
, mFixed(false)
{
MOZ_ASSERT(mTextNode->TextLength() >= mOffset);
}
CompositionTransaction::~CompositionTransaction()
{
}
@@ -69,38 +103,41 @@ CompositionTransaction::DoTransaction()
NS_ENSURE_TRUE(selCon, NS_ERROR_NOT_INITIALIZED);
// Advance caret: This requires the presentation shell to get the selection.
if (mReplaceLength == 0) {
nsresult rv = mTextNode->InsertData(mOffset, mStringToInsert);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
- mRangeUpdater->SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
+ mEditorBase->RangeUpdaterRef().
+ SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
} else {
uint32_t replaceableLength = mTextNode->TextLength() - mOffset;
nsresult rv =
mTextNode->ReplaceData(mOffset, mReplaceLength, mStringToInsert);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
- mRangeUpdater->SelAdjDeleteText(mTextNode, mOffset, mReplaceLength);
- mRangeUpdater->SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
+ mEditorBase->RangeUpdaterRef().
+ SelAdjDeleteText(mTextNode, mOffset, mReplaceLength);
+ mEditorBase->RangeUpdaterRef().
+ SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
// If IME text node is multiple node, ReplaceData doesn't remove all IME
// text. So we need remove remained text into other text node.
if (replaceableLength < mReplaceLength) {
int32_t remainLength = mReplaceLength - replaceableLength;
nsCOMPtr<nsINode> node = mTextNode->GetNextSibling();
while (node && node->IsNodeOfType(nsINode::eTEXT) &&
remainLength > 0) {
Text* text = static_cast<Text*>(node.get());
uint32_t textLength = text->TextLength();
text->DeleteData(0, remainLength);
- mRangeUpdater->SelAdjDeleteText(text, 0, remainLength);
+ mEditorBase->RangeUpdaterRef().SelAdjDeleteText(text, 0, remainLength);
remainLength -= textLength;
node = node->GetNextSibling();
}
}
}
nsresult rv = SetSelectionForRanges();
NS_ENSURE_SUCCESS(rv, rv);
--- a/editor/libeditor/CompositionTransaction.h
+++ b/editor/libeditor/CompositionTransaction.h
@@ -12,48 +12,59 @@
#define NS_IMETEXTTXN_IID \
{ 0xb391355d, 0x346c, 0x43d1, \
{ 0x85, 0xed, 0x9e, 0x65, 0xbe, 0xe7, 0x7e, 0x48 } }
namespace mozilla {
class EditorBase;
-class RangeUpdater;
class TextComposition;
class TextRangeArray;
namespace dom {
class Text;
} // namespace dom
/**
* CompositionTransaction stores all edit for a composition, i.e.,
* from compositionstart event to compositionend event. E.g., inserting a
* composition string, modifying the composition string or its IME selection
* ranges and commit or cancel the composition.
*/
class CompositionTransaction final : public EditTransactionBase
{
+protected:
+ CompositionTransaction(EditorBase& aEditorBase,
+ const nsAString& aStringToInsert,
+ dom::Text& aTextNode,
+ uint32_t aOffset);
+
public:
NS_DECLARE_STATIC_IID_ACCESSOR(NS_IMETEXTTXN_IID)
/**
+ * Creates a composition transaction. aEditorBase must not return from
+ * GetComposition() while calling this method. Note that this method will
+ * update text node information of aEditorBase.mComposition.
+ *
* @param aEditorBase The editor which has composition.
* @param aStringToInsert The new composition string to insert. This may
* be different from actual composition string.
* E.g., password editor can hide the character
* with a different character.
- * @param aTextComposition The composition.
- * @param aRangeUpdater The range updater.
+ * @param aTextNode The text node which will have aStringToInsert.
+ * @param aOffset The offset in aTextNode where aStringToInsert
+ * will be inserted.
*/
- CompositionTransaction(EditorBase& aEditorBase,
- const nsAString& aStringToInsert,
- const TextComposition& aTextComposition,
- RangeUpdater* aRangeUpdater);
+ static already_AddRefed<CompositionTransaction>
+ Create(EditorBase& aEditorBase,
+ const nsAString& aStringToInsert,
+ dom::Text& aTextNode,
+ uint32_t aOffset);
NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(CompositionTransaction,
EditTransactionBase)
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_EDITTRANSACTIONBASE
@@ -84,18 +95,16 @@ private:
RefPtr<TextRangeArray> mRanges;
// The text to insert into mTextNode at mOffset.
nsString mStringToInsert;
// The editor, which is used to get the selection controller.
RefPtr<EditorBase> mEditorBase;
- RangeUpdater* mRangeUpdater;
-
bool mFixed;
};
NS_DEFINE_STATIC_IID_ACCESSOR(CompositionTransaction, NS_IMETEXTTXN_IID)
} // namespace mozilla
#endif // #ifndef CompositionTransaction_h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2779,24 +2779,25 @@ EditorBase::InsertTextIntoTextNodeImpl(c
RefPtr<EditTransactionBase> transaction;
bool isIMETransaction = false;
RefPtr<Text> insertedTextNode = &aTextNode;
int32_t insertedOffset = aOffset;
// aSuppressIME is used when editor must insert text, yet this text is not
// part of the current IME operation. Example: adjusting whitespace around an
// IME insertion.
if (ShouldHandleIMEComposition() && !aSuppressIME) {
- // XXX Here is still ugly. This should be fixed by a follow up bug.
- mComposition->WillCreateCompositionTransaction(&aTextNode, aOffset);
- transaction = CreateTxnForComposition(aStringToInsert);
- mComposition->DidCreateCompositionTransaction(aStringToInsert);
+ transaction =
+ CompositionTransaction::Create(*this, aStringToInsert,
+ aTextNode, aOffset);
isIMETransaction = true;
// All characters of the composition string will be replaced with
// aStringToInsert. So, we need to emulate to remove the composition
// string.
+ // FYI: The text node information in mComposition has been updated by
+ // CompositionTransaction::Create().
insertedTextNode = mComposition->GetContainerTextNode();
insertedOffset = mComposition->XPOffsetInTextNode();
} else {
transaction =
InsertTextTransaction::Create(*this, aStringToInsert, aTextNode, aOffset);
}
// Let listeners know what's up
@@ -4650,30 +4651,16 @@ EditorBase::CreateTxnForDeleteNode(nsINo
// This should be OK because if currently it cannot delete the node,
// it should never be able to undo/redo.
if (!deleteNodeTransaction->CanDoIt()) {
return nullptr;
}
return deleteNodeTransaction.forget();
}
-already_AddRefed<CompositionTransaction>
-EditorBase::CreateTxnForComposition(const nsAString& aStringToInsert)
-{
- MOZ_ASSERT(mComposition);
- MOZ_ASSERT(mComposition->GetContainerTextNode());
- // During handling IME composition, mComposition must have been initialized.
- // TODO: We can simplify CompositionTransaction::Init() with TextComposition
- // class.
- RefPtr<CompositionTransaction> transaction =
- new CompositionTransaction(*this, aStringToInsert, *mComposition,
- &mRangeUpdater);
- return transaction.forget();
-}
-
already_AddRefed<AddStyleSheetTransaction>
EditorBase::CreateTxnForAddStyleSheet(StyleSheet* aSheet)
{
RefPtr<AddStyleSheetTransaction> transaction =
new AddStyleSheetTransaction(*this, aSheet);
return transaction.forget();
}
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -577,22 +577,16 @@ protected:
already_AddRefed<EditTransactionBase>
CreateTxnForDeleteRange(nsRange* aRangeToDelete,
EDirection aAction,
nsINode** aRemovingNode,
int32_t* aOffset,
int32_t* aLength);
/**
- * Never returns null.
- */
- already_AddRefed<mozilla::CompositionTransaction>
- CreateTxnForComposition(const nsAString& aStringToInsert);
-
- /**
* Create a transaction for adding a style sheet.
*/
already_AddRefed<mozilla::AddStyleSheetTransaction>
CreateTxnForAddStyleSheet(StyleSheet* aSheet);
/**
* Create a transaction for removing a style sheet.
*/