Bug 1582010 - Make `IMEContentObserver` always recompute selection range after sending text change notification r=m_kato a=lizzard
`IMEContentObserver` sends selection change notification only when it receives
a DOM `Selection` change notification received. However, selection range in
plaintext offset may be changed when changing previous content of caret is
changed. In this case, currently we notify IME of only text change and
that causes IME keep caching selection offsets in previous content. This
causes IME queries character rect out of bounds. Therefore, even if
`IMEContentObserver` hasn't received DOM `Selection` change notification,
it should recompute selection range and if it's changed, it should notify
IME of selection change too.
Differential Revision:
https://phabricator.services.mozilla.com/D46251
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -1679,24 +1679,29 @@ IMEContentObserver::IMENotificationSende
if (observer->mNeedsToNotifyIMEOfTextChange) {
observer->mNeedsToNotifyIMEOfTextChange = false;
SendTextChange();
}
// If a text change notification causes another text change again, we should
// notify IME of that before sending a selection change notification.
+ // Otherwise, even if the observer hasn't received selection change, let's
+ // try to send selection change notification to IME because selection
+ // start offset may be changed if the previous contents of selection start
+ // are changed. For example, when previous `<p>` element of another `<p>`
+ // element which contains caret is removed by a DOM mutation, selection
+ // change event won't be fired, but selection start offset should be
+ // decreased by the length of removed `<p>` element.
if (!observer->mNeedsToNotifyIMEOfTextChange) {
// Be aware, PuppetWidget depends on the order of this. A selection change
// notification should not be sent before a text change notification because
// PuppetWidget shouldn't query new text content every selection change.
- if (observer->mNeedsToNotifyIMEOfSelectionChange) {
- observer->mNeedsToNotifyIMEOfSelectionChange = false;
- SendSelectionChange();
- }
+ observer->mNeedsToNotifyIMEOfSelectionChange = false;
+ SendSelectionChange();
}
// If a text change notification causes another text change again or a
// selection change notification causes either a text change or another
// selection change, we should notify IME of those before sending a position
// change notification.
if (!observer->mNeedsToNotifyIMEOfTextChange &&
!observer->mNeedsToNotifyIMEOfSelectionChange) {
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -8810,25 +8810,56 @@ async function runIMEContentObserverTest
await waitNotifications;
ensureToRemovePrecedingPositionChangeNotification();
is(aElement.innerHTML, "<div>1<div>2<div>39</div></div></div>", description + " should remove '</div>4</div>5<div>6<div>7</div>8</div>'");
// It causes removing '</div>4</div>5<div>6<div>7</div>8</div>' and inserting '<div>36<div>7</div>8</div>'.
checkTextChangeNotification(notifications[0], description, { offset: getNativeText("\n1\n2\n3").length + offsetAtStart, removedLength: getNativeText("45\n6\n789").length, addedLength: getNativeText("9").length });
checkSelectionChangeNotification(notifications[1], description, { offset: getNativeText("\n1\n2\n3").length + offsetAtStart, text: "" });
checkPositionChangeNotification(notifications[2], description);
dumpUnexpectedNotifications(description, 3);
+
+ // "<p>abc</p><p><br></p><p>{<br>}</p>" and removing second paragraph with DOM API
+ aElement.innerHTML = "<p>abc</p><p><br></p><p><br></p>";
+ sel.collapse(aElement.firstChild.nextSibling.nextSibling, 0);
+ await flushNotifications();
+ description = aDescription + "deleting previous paragraph with DOM API";
+ waitNotifications = promiseReceiveNotifications();
+ synthesizeKey("KEY_Unidentified", { code: "" }, win, callback); // For setting the callback to recode notifications
+ aElement.firstChild.nextSibling.remove();
+ await waitNotifications;
+ ensureToRemovePrecedingPositionChangeNotification();
+ is(aElement.innerHTML, "<p>abc</p><p><br></p>", description + " the second paragraph should've been removed");
+ checkTextChangeNotification(notifications[0], description, { offset: getNativeText("\nabc").length + offsetAtStart, removedLength: getNativeText("\n\n").length, addedLength: 0 });
+ checkSelectionChangeNotification(notifications[1], description, { offset: getNativeText("\nabc\n").length + offsetAtStart, text: "" });
+ checkPositionChangeNotification(notifications[2], description);
+ dumpUnexpectedNotifications(description, 3);
+
+ // "<p>abc</p><p>{<br>}</p><p><br></p>" and removing last paragraph with DOM API
+ aElement.innerHTML = "<p>abc</p><p><br></p><p><br></p>";
+ sel.collapse(aElement.firstChild.nextSibling, 0);
+ await flushNotifications();
+ description = aDescription + "deleting next paragraph with DOM API";
+ waitNotifications = promiseReceiveNotifications();
+ synthesizeKey("KEY_Unidentified", { code: "" }, win, callback); // For setting the callback to recode notifications
+ aElement.firstChild.nextSibling.nextSibling.remove();
+ await waitNotifications;
+ ensureToRemovePrecedingPositionChangeNotification();
+ is(aElement.innerHTML, "<p>abc</p><p><br></p>", description + " the last paragraph should've been removed");
+ checkTextChangeNotification(notifications[0], description, { offset: getNativeText("\nabc\n\n").length + offsetAtStart, removedLength: getNativeText("\n\n").length, addedLength: 0 });
+ checkPositionChangeNotification(notifications[1], description);
+ dumpUnexpectedNotifications(description, 2);
}
await testWithPlaintextEditor("runIMEContentObserverTest with input element: ", input, false);
await testWithPlaintextEditor("runIMEContentObserverTest with textarea element: ", textarea, true);
await testWithHTMLEditor("runIMEContentObserverTest with contenteditable (defaultParagraphSeparator is br): ", contenteditable, "br");
await testWithHTMLEditor("runIMEContentObserverTest with contenteditable (defaultParagraphSeparator is p): ", contenteditable, "p");
await testWithHTMLEditor("runIMEContentObserverTest with contenteditable (defaultParagraphSeparator is div): ", contenteditable, "div");
// XXX Due to the difference of HTML editor behavior between designMode and contenteditable,
- // testWithHTMLEditor() gets some unexpected behavior. However, IMEContentObserveri is
+ // testWithHTMLEditor() gets some unexpected behavior. However, IMEContentObserver is
// not depend on editor's detail. So, we should investigate this issue later. It's not
// so important for now.
// await testWithHTMLEditor("runIMEContentObserverTest in designMode (defaultParagraphSeparator is br): ", iframe2.contentDocument.body, "br");
// await testWithHTMLEditor("runIMEContentObserverTest in designMode (defaultParagraphSeparator is p): ", iframe2.contentDocument.body, "p");
// await testWithHTMLEditor("runIMEContentObserverTest in designMode (defaultParagraphSeparator is div): ", iframe2.contentDocument.body, "div");
}
async function runPasswordMaskDelayTest() {