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

--HG--
extra : rebase_source : d468a0fc997c99f78f7eb46451082e7f1e052890
This commit is contained in:
Masayuki Nakano 2017-12-15 18:26:37 +09:00
parent 7443737c19
commit a5bd93a76e
5 changed files with 89 additions and 74 deletions

View File

@ -146,11 +146,12 @@ public:
/**
* 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;
}
/**
@ -236,14 +237,21 @@ public:
};
/**
* 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;
@ -253,30 +261,10 @@ public:
}
#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.");

View File

@ -21,18 +21,52 @@ 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);
@ -74,7 +108,8 @@ CompositionTransaction::DoTransaction()
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 =
@ -82,8 +117,10 @@ CompositionTransaction::DoTransaction()
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.
@ -95,7 +132,7 @@ CompositionTransaction::DoTransaction()
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();
}

View File

@ -17,7 +17,6 @@
namespace mozilla {
class EditorBase;
class RangeUpdater;
class TextComposition;
class TextRangeArray;
@ -33,22 +32,34 @@ class Text;
*/
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)
@ -89,8 +100,6 @@ private:
// The editor, which is used to get the selection controller.
RefPtr<EditorBase> mEditorBase;
RangeUpdater* mRangeUpdater;
bool mFixed;
};

View File

@ -2784,14 +2784,15 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert,
// 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 {
@ -4649,20 +4650,6 @@ EditorBase::CreateTxnForDeleteNode(nsINode* aNode)
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)
{

View File

@ -581,12 +581,6 @@ protected:
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.
*/