Bug 1250823 part 2 - IMEContentObserver should cache adding ranges as a range during document change r?smaug
Between nsIDocumentObserver::BeginUpdate() and nsIDocumentObserver::EndUpdate(), IMEContentObserver can cache added nodes as a range if they are consecutive nodes. Even if a node is removed, a data node is changed or attribute is changed unexpectedly, IMEContentObserver can post text change of the added node range and handle it normally.
MozReview-Commit-ID: IttDHBkr92Y
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -116,16 +116,20 @@ public:
};
/******************************************************************************
* mozilla::IMEContentObserver
******************************************************************************/
NS_IMPL_CYCLE_COLLECTION_CLASS(IMEContentObserver)
+// Note that we don't need to add mFirstAddedNodeContainer nor
+// mLastAddedNodeContainer to cycle collection because they are non-null only
+// during short time and shouldn't be touched while they are non-null.
+
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IMEContentObserver)
nsAutoScriptBlocker scriptBlocker;
tmp->NotifyIMEOfBlur();
tmp->UnregisterObservers();
NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelection)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mRootContent)
@@ -163,17 +167,19 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
NS_INTERFACE_MAP_ENTRY(nsIEditorObserver)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsISelectionListener)
NS_INTERFACE_MAP_END
NS_IMPL_CYCLE_COLLECTING_ADDREF(IMEContentObserver)
NS_IMPL_CYCLE_COLLECTING_RELEASE(IMEContentObserver)
IMEContentObserver::IMEContentObserver()
- : mESM(nullptr)
+ : mFirstAddedNodeOffset(0)
+ , mLastAddedNodeOffset(0)
+ , mESM(nullptr)
, mIMENotificationRequests(nullptr)
, mSuppressNotifications(0)
, mPreCharacterDataChangeLength(-1)
, mSendingNotification(NOTIFY_IME_OF_NOTHING)
, mIsObserving(false)
, mIMEHasFocus(false)
, mNeedsToNotifyIMEOfFocusSet(false)
, mNeedsToNotifyIMEOfTextChange(false)
@@ -928,16 +934,23 @@ IMEContentObserver::CharacterDataWillCha
"mPreCharacterDataChangeLength");
if (!NeedsTextChangeNotification()) {
return;
}
mEndOfAddedTextCache.Clear();
mStartOfRemovingTextRangeCache.Clear();
+
+ // Although we don't assume this change occurs while this is storing
+ // the range of added consecutive nodes, if it actually happens, we need to
+ // flush them since this change may occur before or in the range. So, it's
+ // safe to flush pending computation of mTextChangeData before handling this.
+ MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+
mPreCharacterDataChangeLength =
ContentEventHandler::GetNativeTextLength(aContent, aInfo->mChangeStart,
aInfo->mChangeEnd);
MOZ_ASSERT(mPreCharacterDataChangeLength >=
aInfo->mChangeEnd - aInfo->mChangeStart,
"The computed length must be same as or larger than XP length");
}
@@ -950,16 +963,18 @@ IMEContentObserver::CharacterDataChanged
"character data changed for non-text node");
if (!NeedsTextChangeNotification()) {
return;
}
mEndOfAddedTextCache.Clear();
mStartOfRemovingTextRangeCache.Clear();
+ MOZ_ASSERT(!HasAddedNodesDuringDocumentChange(),
+ "The stored range should be flushed before actually the data is changed");
int64_t removedLength = mPreCharacterDataChangeLength;
mPreCharacterDataChangeLength = -1;
MOZ_ASSERT(removedLength >= 0,
"mPreCharacterDataChangeLength should've been set by "
"CharacterDataWillChange()");
@@ -994,16 +1009,52 @@ IMEContentObserver::NotifyContentAdded(n
int32_t aEndIndex)
{
if (!NeedsTextChangeNotification()) {
return;
}
mStartOfRemovingTextRangeCache.Clear();
+ // If it's in a document change, nodes are added consecutively. Therefore,
+ // if we cache the first node and the last node, we need to compute the
+ // range once.
+ // FYI: This is not true if the change caused by an operation in the editor.
+ if (IsInDocumentChange()) {
+ // Now, mEndOfAddedTextCache may be invalid if node is added before
+ // the last node in mEndOfAddedTextCache. Clear it.
+ mEndOfAddedTextCache.Clear();
+ if (!HasAddedNodesDuringDocumentChange()) {
+ mFirstAddedNodeContainer = mLastAddedNodeContainer = aContainer;
+ mFirstAddedNodeOffset = aStartIndex;
+ mLastAddedNodeOffset = aEndIndex;
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::NotifyContentAdded(), starts to store "
+ "consecutive added nodes", this));
+ return;
+ }
+ // If first node being added is not next node of the last node,
+ // notify IME of the previous range first, then, restart to cache the
+ // range.
+ if (NS_WARN_IF(!IsNextNodeOfLastAddedNode(aContainer, aStartIndex))) {
+ // Flush the old range first.
+ MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+ mFirstAddedNodeContainer = aContainer;
+ mFirstAddedNodeOffset = aStartIndex;
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::NotifyContentAdded(), starts to store "
+ "consecutive added nodes", this));
+ }
+ mLastAddedNodeContainer = aContainer;
+ mLastAddedNodeOffset = aEndIndex;
+ return;
+ }
+ MOZ_ASSERT(!HasAddedNodesDuringDocumentChange(),
+ "The cache should be cleared when document change finished");
+
uint32_t offset = 0;
nsresult rv = NS_OK;
if (!mEndOfAddedTextCache.Match(aContainer, aStartIndex)) {
mEndOfAddedTextCache.Clear();
rv = ContentEventHandler::GetFlatTextLengthInRange(
NodePosition(mRootContent, 0),
NodePositionBefore(aContainer, aStartIndex),
mRootContent, &offset, LINE_BREAK_TYPE_NATIVE);
@@ -1069,16 +1120,17 @@ IMEContentObserver::ContentRemoved(nsIDo
int32_t aIndexInContainer,
nsIContent* aPreviousSibling)
{
if (!NeedsTextChangeNotification()) {
return;
}
mEndOfAddedTextCache.Clear();
+ MaybeNotifyIMEOfAddedTextDuringDocumentChange();
nsINode* containerNode = NODE_FROM(aContainer, aDocument);
uint32_t offset = 0;
nsresult rv = NS_OK;
if (!mStartOfRemovingTextRangeCache.Match(containerNode, aIndexInContainer)) {
// At removing a child node of aContainer, we need the line break caused
// by open tag of aContainer. Be careful when aIndexInContainer is 0.
@@ -1154,16 +1206,19 @@ IMEContentObserver::AttributeChanged(nsI
mEndOfAddedTextCache.Clear();
mStartOfRemovingTextRangeCache.Clear();
uint32_t postAttrChangeLength =
ContentEventHandler::GetNativeTextLengthBefore(aElement, mRootContent);
if (postAttrChangeLength == mPreAttrChangeLength) {
return;
}
+ // First, compute text range which were added during a document change.
+ MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+ // Then, compute the new text changed caused by this attribute change.
uint32_t start;
nsresult rv =
ContentEventHandler::GetFlatTextLengthInRange(
NodePosition(mRootContent, 0),
NodePositionBefore(aElement, 0),
mRootContent, &start, LINE_BREAK_TYPE_NATIVE);
if (NS_WARN_IF(NS_FAILED(rv))) {
return;
@@ -1172,25 +1227,176 @@ IMEContentObserver::AttributeChanged(nsI
TextChangeData data(start, start + mPreAttrChangeLength,
start + postAttrChangeLength,
IsEditorHandlingEventForComposition(),
IsEditorComposing());
MaybeNotifyIMEOfTextChange(data);
}
void
+IMEContentObserver::ClearAddedNodesDuringDocumentChange()
+{
+ mFirstAddedNodeContainer = mLastAddedNodeContainer = nullptr;
+ mFirstAddedNodeOffset = mLastAddedNodeOffset = 0;
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::ClearAddedNodesDuringDocumentChange()"
+ ", finished storing consecutive nodes", this));
+}
+
+// static
+nsIContent*
+IMEContentObserver::GetChildNode(nsINode* aParent, int32_t aOffset)
+{
+ if (!aParent->HasChildren() || aOffset < 0 ||
+ aOffset >= static_cast<int32_t>(aParent->Length())) {
+ return nullptr;
+ }
+ if (!aOffset) {
+ return aParent->GetFirstChild();
+ }
+ if (aOffset == static_cast<int32_t>(aParent->Length() - 1)) {
+ return aParent->GetLastChild();
+ }
+ return aParent->GetChildAt(aOffset);
+}
+
+bool
+IMEContentObserver::IsNextNodeOfLastAddedNode(nsINode* aParent,
+ int32_t aOffset) const
+{
+ MOZ_ASSERT(aParent);
+ MOZ_ASSERT(aOffset >= 0 &&
+ aOffset <= static_cast<int32_t>(aParent->Length()));
+ MOZ_ASSERT(mRootContent);
+ MOZ_ASSERT(HasAddedNodesDuringDocumentChange());
+
+ // If the parent node isn't changed, we can check it only with offset.
+ if (aParent == mLastAddedNodeContainer) {
+ if (NS_WARN_IF(mLastAddedNodeOffset != aOffset)) {
+ return false;
+ }
+ return true;
+ }
+
+ // If the parent node is changed, that means that given offset should be the
+ // last added node not having next sibling.
+ if (NS_WARN_IF(mLastAddedNodeOffset !=
+ static_cast<int32_t>(mLastAddedNodeContainer->Length()))) {
+ return false;
+ }
+
+ // If the node is aParent is a descendant of mLastAddedNodeContainer,
+ // aOffset should be 0.
+ if (mLastAddedNodeContainer == aParent->GetParent()) {
+ if (NS_WARN_IF(aOffset)) {
+ return false;
+ }
+ return true;
+ }
+
+ // Otherwise, we need to check it even with slow path.
+ nsIContent* lastAddedContent =
+ GetChildNode(mLastAddedNodeContainer, mLastAddedNodeOffset - 1);
+ if (NS_WARN_IF(!lastAddedContent)) {
+ return false;
+ }
+
+ nsIContent* nextContentOfLastAddedContent =
+ lastAddedContent->GetNextNode(mRootContent->GetParentNode());
+ if (NS_WARN_IF(!nextContentOfLastAddedContent)) {
+ return false;
+ }
+
+ nsIContent* startContent = GetChildNode(aParent, aOffset);
+ if (NS_WARN_IF(!startContent) ||
+ NS_WARN_IF(nextContentOfLastAddedContent != startContent)) {
+ return false;
+ }
+#ifdef DEBUG
+ NS_WARNING_ASSERTION(
+ !aOffset || aOffset == static_cast<int32_t>(aParent->Length() - 1),
+ "Used slow path for aParent");
+ NS_WARNING_ASSERTION(
+ !(mLastAddedNodeOffset - 1) ||
+ mLastAddedNodeOffset ==
+ static_cast<int32_t>(mLastAddedNodeContainer->Length()),
+ "Used slow path for mLastAddedNodeContainer");
+#endif // #ifdef DEBUG
+ return true;
+}
+
+void
+IMEContentObserver::MaybeNotifyIMEOfAddedTextDuringDocumentChange()
+{
+ if (!HasAddedNodesDuringDocumentChange()) {
+ return;
+ }
+
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::MaybeNotifyIMEOfAddedTextDuringDocumentChange()"
+ ", flushing stored consecutive nodes", this));
+
+ // Notify IME of text change which is caused by added nodes now.
+
+ // First, compute offset of start of first added node from start of the
+ // editor.
+ uint32_t offset;
+ nsresult rv =
+ ContentEventHandler::GetFlatTextLengthInRange(
+ NodePosition(mRootContent, 0),
+ NodePosition(mFirstAddedNodeContainer,
+ mFirstAddedNodeOffset),
+ mRootContent, &offset, LINE_BREAK_TYPE_NATIVE);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ ClearAddedNodesDuringDocumentChange();
+ return;
+ }
+
+ // Next, compute the text length of added nodes.
+ uint32_t length;
+ rv =
+ ContentEventHandler::GetFlatTextLengthInRange(
+ NodePosition(mFirstAddedNodeContainer,
+ mFirstAddedNodeOffset),
+ NodePosition(mLastAddedNodeContainer,
+ mLastAddedNodeOffset),
+ mRootContent, &length, LINE_BREAK_TYPE_NATIVE);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ ClearAddedNodesDuringDocumentChange();
+ return;
+ }
+
+ // Finally, try to notify IME of the range.
+ TextChangeData data(offset, offset, offset + length,
+ IsEditorHandlingEventForComposition(),
+ IsEditorComposing());
+ MaybeNotifyIMEOfTextChange(data);
+ ClearAddedNodesDuringDocumentChange();
+}
+
+void
IMEContentObserver::BeginDocumentUpdate()
{
- // TODO: Implement this later.
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::BeginDocumentUpdate(), "
+ "HasAddedNodesDuringDocumentChange()=%s",
+ this, ToChar(HasAddedNodesDuringDocumentChange())));
+
+ MOZ_ASSERT(!HasAddedNodesDuringDocumentChange());
}
void
IMEContentObserver::EndDocumentUpdate()
{
- // TODO: Implement this later.
+ MOZ_LOG(sIMECOLog, LogLevel::Debug,
+ ("0x%p IMEContentObserver::EndDocumentUpdate(), "
+ "HasAddedNodesDuringDocumentChange()=%s",
+ this, ToChar(HasAddedNodesDuringDocumentChange())));
+
+ MaybeNotifyIMEOfAddedTextDuringDocumentChange();
}
void
IMEContentObserver::SuppressNotifyingIME()
{
mSuppressNotifications++;
MOZ_LOG(sIMECOLog, LogLevel::Debug,
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -177,21 +177,71 @@ private:
void OnIMEReceivedFocus();
void Clear();
bool IsObservingContent(nsPresContext* aPresContext,
nsIContent* aContent) const;
bool IsReflowLocked() const;
bool IsSafeToNotifyIME() const;
bool IsEditorComposing() const;
+ /**
+ * nsINode::GetChildAt() is slow. So, this avoids to use it if it's
+ * first child or last child of aParent.
+ */
+ static nsIContent* GetChildNode(nsINode* aParent, int32_t aOffset);
+
// Following methods are called by DocumentObserver when
// beginning to update the contents and ending updating the contents.
void BeginDocumentUpdate();
void EndDocumentUpdate();
+ // Following methods manages added nodes during a document change.
+
+ /**
+ * MaybeNotifyIMEOfAddedTextDuringDocumentChange() may send text change
+ * notification caused by the nodes added between mFirstAddedNodeOffset in
+ * mFirstAddedNodeContainer and mLastAddedNodeOffset in
+ * mLastAddedNodeContainer and forgets the range.
+ */
+ void MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+
+ /**
+ * IsInDocumentChange() returns true while the DOM tree is being modified
+ * with mozAutoDocUpdate. E.g., it's being modified by setting innerHTML or
+ * insertAdjacentHTML(). This returns false when user types something in
+ * the focused editor editor.
+ */
+ bool IsInDocumentChange() const
+ {
+ return mDocumentObserver && mDocumentObserver->IsUpdating();
+ }
+
+ /**
+ * Forget the range of added nodes during a document change.
+ */
+ void ClearAddedNodesDuringDocumentChange();
+
+ /**
+ * HasAddedNodesDuringDocumentChange() returns true when this stores range
+ * of nodes which were added into the DOM tree during a document change but
+ * have not been sent to IME. Note that this should always return false when
+ * IsInDocumentChange() returns false.
+ */
+ bool HasAddedNodesDuringDocumentChange() const
+ {
+ return mFirstAddedNodeContainer && mLastAddedNodeContainer;
+ }
+
+ /**
+ * Returns true if the node at aOffset in aParent is next node of the node at
+ * mLastAddedNodeOffset in mLastAddedNodeContainer in pre-order tree
+ * traversal of the DOM.
+ */
+ bool IsNextNodeOfLastAddedNode(nsINode* aParent, int32_t aOffset) const;
+
void PostFocusSetNotification();
void MaybeNotifyIMEOfFocusSet();
void PostTextChangeNotification();
void MaybeNotifyIMEOfTextChange(const TextChangeDataBase& aTextChangeData);
void CancelNotifyingIMEOfTextChange();
void PostSelectionChangeNotification();
void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition,
bool aCausedBySelectionEvent,
@@ -409,16 +459,37 @@ private:
// handled by the editor and no other mutation (e.g., removing node)
// occur.
FlatTextCache mEndOfAddedTextCache;
// mStartOfRemovingTextRangeCache caches text length from the start of content
// to the start of the last removed content only while an edit action is being
// handled by the editor and no other mutation (e.g., adding node) occur.
FlatTextCache mStartOfRemovingTextRangeCache;
+ // mFirstAddedNodeContainer is parent node of first added node in current
+ // document change. So, this is not nullptr only when a node was added
+ // during a document change and the change has not been included into
+ // mTextChangeData yet.
+ // Note that this shouldn't be in cycle collection since this is not nullptr
+ // only during a document change.
+ nsCOMPtr<nsINode> mFirstAddedNodeContainer;
+ // mLastAddedNodeContainer is parent node of last added node in current
+ // document change. So, this is not nullptr only when a node was added
+ // during a document change and the change has not been included into
+ // mTextChangeData yet.
+ // Note that this shouldn't be in cycle collection since this is not nullptr
+ // only during a document change.
+ nsCOMPtr<nsINode> mLastAddedNodeContainer;
+ // mFirstAddedNodeOffset is offset of first added node in
+ // mFirstAddedNodeContainer.
+ int32_t mFirstAddedNodeOffset;
+ // mLastAddedNodeOffset is offset of *after* last added node in
+ // mLastAddedNodeContainer. I.e., the index of last added node + 1.
+ int32_t mLastAddedNodeOffset;
+
TextChangeData mTextChangeData;
// mSelectionData is the last selection data which was notified. The
// selection information is modified by UpdateSelectionCache(). The reason
// of the selection change is modified by MaybeNotifyIMEOfSelectionChange().
SelectionChangeData mSelectionData;
EventStateManager* mESM;
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -7505,23 +7505,17 @@ function* runIMEContentObserverTest()
yield flushNotifications();
// "a[]"
var description = aDescription + "typing 'a'";
notifications = [];
synthesizeKey("a", { code: "KeyA" }, win, callback);
yield waitUntilNotificationsReceived();
ensureToRemovePrecedingPositionChangeNotification();
- if (isDefaultParagraphSeparatorBlock) {
- // XXX This must detect a bug. The offset of inserting "a" into the first block should be
- // after the line breaker caused by the first block.
- checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: 0, addedLength: 1 });
- } else {
- checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 0, addedLength: 1 });
- }
+ checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 0, addedLength: 1 });
checkSelectionChangeNotification(notifications[1], description, { offset: 1 + offsetAtContainer, text: "" });
checkPositionChangeNotification(notifications[2], description);
dumpUnexpectedNotifications(description, 3);
// "ab[]"
description = aDescription + "typing 'b'";
notifications = [];
synthesizeKey("b", { code: "KeyB" }, win, callback);
@@ -7609,23 +7603,17 @@ function* runIMEContentObserverTest()
dumpUnexpectedNotifications(description, 3);
// "[]"
description = aDescription + "deleting following 'c' with pressing Delete";
notifications = [];
synthesizeKey("KEY_Delete", { code: "Delete" }, win, callback);
yield waitUntilNotificationsReceived();
ensureToRemovePrecedingPositionChangeNotification();
- if (isDefaultParagraphSeparatorBlock) {
- // XXX Making a block empty causes removing the block once.
- // However, after that, new block is inserted with <br>.
- checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: getNativeText("\nc").length, addedLength: kLFLen * 2 });
- } else {
- checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 1, addedLength: kLFLen });
- }
+ checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 1, addedLength: kLFLen });
checkPositionChangeNotification(notifications[1], description);
dumpUnexpectedNotifications(description, 2);
// "abc[]"
synthesizeKey("a", { code: "KeyA" }, win, callback);
synthesizeKey("b", { code: "KeyB" }, win, callback);
synthesizeKey("c", { code: "KeyC" }, win, callback);
yield flushNotifications();