1203364 - IMEContentObserver doesn't need to manage the reason of selection change because the last reason is enough for widget

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Currently, IMEContentObserver manages the reason of selection change itself. However, it does NOT indicate the last selection change reason. It doesn't make sense except bug 1184890. In current design, widget can ignore unexpected selection change if it occurs before the latest expected selection change.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff6b6b2c661

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=942b60bdb3f8

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: IMEContentObserver should notify IME of selection change with the latest change reason

Let's notify IME of the latest selection with the latest selection change reason. This makes IMEContentObserver simpler and makes sense because if we need to notify IME of omitted selection change reason, we need to notify IME of the omitted selection change separately.

Although, we might have some problem around this if selection is changed from compositionupdate listener. But we should investigate it later since it's not realistic scenario.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8660629 [details] [diff] [review]
IMEContentObserver should notify IME of selection change with the latest change reason

>-  // mSelectionData is the last selection data which was notified.  This is
>-  // modified by UpdateSelectionCache().  Note that mCausedBy* are always
>-  // false.  Do NOT refer them.
>+  // 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 PostSelectionChangeNotification().
No it isn't. PostSelectionChangeNotification does not modify the reason, but IMEContentObserver::MaybeNotifyIMEOfSelectionChange

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/fe70add404858840b24f81d522033959e95b5f53
changeset:  fe70add404858840b24f81d522033959e95b5f53
user:       Masayuki Nakano <[e-mail]>
date:       Mon Sep 14 23:28:43 2015 +0900
description:
Bug 1203364 IMEContentObserver should notify IME of selection change with the latest change reason r=smaug

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fe70add40485