When we return `TS_E_NOLAYOUT` from `ITextStoreACP::GetTextExt()`,
WeChat Input Method does not handle the error correctly and shows its
UI to the top-left corner of the screen. Therefore, we need not to
return `TS_E_NOLAYOUT` if it's the active TIP.
However, I found 2 extant bugs and they block fixing this bug.
First one is, `TSFTextStore::MaybeHackNoErrorLayoutBugs()` does nothing
if `IsHandlingCompositionInContent()` return `false`. [1] This means
that we don't hack `GetTextExt()` behavior when the composition starts
and not yet dispatched the first event. So, this causes temporarily
appear the UI in odd place, like top-left corner of the screen even with
some other IMEs.
The other one is, `TSFTextStore::GetTextExt()` may query a line break
rect when the user starts composition at end of a line. [2] So, only
when starting composition, the UI may (maybe temporarily) appear at
start of next line. Therefore, if the line scanning range in the
original content has a linefeed, we should query a rect before the
first linefeed, i.e., only the text rect in the start line even if it's
0-length. The difference between the caret rect and the new character
rect should be smaller than the difference from the start of the next
line.
1. https://searchfox.org/firefox-main/rev/e28b34ab33dbf49364999070168cbb7e11e8e5bd/widget/windows/TSFTextStore.cpp#2156,2172,2175
2. https://searchfox.org/firefox-main/rev/e28b34ab33dbf49364999070168cbb7e11e8e5bd/widget/windows/TSFTextStore.cpp#2070
Differential Revision: https://phabricator.services.mozilla.com/D303266
While referring an extant item in the pending action array,
`TSFTextStore::PendingActions` shouldn't delete the extant item.
Therefore, while referring an extant item, `TSFTextStore` needs to
lock the array.
Note that removing array items during a lock should be caused by
a malware or a bug of the TIP. Therefore, we don't need to try to
recover the normal condition with complicated handling.
Differential Revision: https://phabricator.services.mozilla.com/D297551
We should use `ContiguousEnumSerializerInclusive` to receive/send an
`enum` value.
`IMENotificationRequests` has an enum whose values are used as bit
field. So, we can use `EnumSet` for it and then, we don't need to write
`ParamTraits` for it directly.
Differential Revision: https://phabricator.services.mozilla.com/D285200
This fixes a regression of bug 1137561. That made `TSFTextStore` stop
dispatching composition events directly. However, its dispatcher sets
`mDeferNotifyingTSFUntilNextUpdate` to notify TSF of anything until
it receives `NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED` since
`GetTextExt()` etc will return error if the layout has not be updated.
So, we started return `E_FAIL` a lot from various `ITextStoreACP`
methods if TSF/TIP queries something immediately after updating
the composition.
Differential Revision: https://phabricator.services.mozilla.com/D271560
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
Although I'm not sure why this breaks the legacy MS-IME for Japanese.
When `InputContext` is set, we may need to emulate focus to recreate
`TSFTextStoreBase` instance. This is not required if the editable state is not
changed, but the check was removed by D242682. This patch restores the check.
However, instead, making it sure to update the document URL and private browsing
mode for the privacy.
Additionally, I found another difference. D242682 made `TSFEmptyTextStore`
starts associating its document manager with the active window. However,
this is not required and that may cause making TSF calling `SetFocus()` of the
document manager even when editable `TSFTextStore` already got focus.
Therefore, this removes the unnecessary new thing too. Therefore,
`TSFUtils::OnFocusChange` skips to disassociate document manager from a
window handle.
Differential Revision: https://phabricator.services.mozilla.com/D245348
Even while IME is disabled (i.e., when non-editable element or nobody has
focus), we'll expose active tab's URL and whether it's in the private browsing
mode via the input scope. Therefore, we need to handle requests for retrieving
attributes. So, we need a TextStore instance for that. However, they may
require special behavior due to its context is marked as empty and not editable.
Unfortunately, `TSFTextStore` is enough complicated and big even after some
features are moved to `TSFUtils` and `TSFTextStoreBase`. Therefore, this patch
creates `TSFEmptyTextStore` to isolate the implementation from `TSFTextStore`.
Differential Revision: https://phabricator.services.mozilla.com/D242682
`TSFEmptyTextStore` will share them. For making the changes for it clearer in
the previous patch, this patch moves them.
Differential Revision: https://phabricator.services.mozilla.com/D242681
We want another `TSFTextStore` class which always has empty content but
allows to reply to retrieving the properties and supports sink.
For avoiding to duplicate some methods and basic parameter checks, we
should have a base class, `TSFTextStoreBase`.
Differential Revision: https://phabricator.services.mozilla.com/D242679
It's too big and the code in it is really complicated. Therefore, I don't
like to make it bigger to implement new `ITfTextStore` subclass. Therefore,
this patch splits the file for `TSFStaticSink`, `InputScopeImpl` (renamed to
`TSFInputScope`) and the others (named `TSFUtils`).
This patch sorts out some logging helper methods/classes. Therefore, this
patch contains some changes in `TSFTextStore.cpp`.
Differential Revision: https://phabricator.services.mozilla.com/D241348
We create a document manager for disabling IME and create a context for it.
However, we don't push the context into the document manager. Therefore,
TSF cannot work with the context unless an editable element has focus.
Differential Revision: https://phabricator.services.mozilla.com/D240969
By default, query content allows to flushing layout. In most cases, this should
not cause recursive call of the caching content/selection methods. However,
according to a crash report, this may occur with no chance to avoid the loop
from `TSFTextStore` point of view. Fortunately, the recursive call may happen
only when an element in chrome in the main process has focus. Therefore, I
think that we can keep current `MOZ_DIAGNOSTIC_ASSERT`s in the methods, but the
nested call should not allow to flush pending layout to avoid an infinite loop
in the release channel.
Differential Revision: https://phabricator.services.mozilla.com/D209992
It requires selection, but it may be called during `SelectionRef()` is called
by another method. In that case, it should do nothing and `SelectionRef()`
should call `CreateNativeCaret` when it gets the result.
Depends on D209990
Differential Revision: https://phabricator.services.mozilla.com/D209991
Currently, we try to commit composition only when we know there is a composition
in `TSFTextStore`. However, according to the crash report, TIP may have a
hidden composition and may fail to manage ref-count of something when we
disassociate IM context from the window when they have a hidden composition.
Therefore, we could avoid the crash if we try to commit all composition in the
all contexts.
Note that a document manager can have at most 2 contexts, according to MSDN [1].
1. https://learn.microsoft.com/en-us/windows/win32/api/msctf/nf-msctf-itfdocumentmgr-push
Differential Revision: https://phabricator.services.mozilla.com/D209993
It'll be notify IMEs on Windows of text/selection with only LFs instead of
CRLFs because the conversion cost from LFs to CRLFs is serious for DOM tree
change performance and Chrome also uses only LFs too.
However, we know some old TIPs inserted CRLFs. TIPs should use emulating
`Enter` key press instead for the issue, "insert paragraph" vs.
"insert line break" in rich text editors, some IMEs may still insert CRLFs
directly. However, nobody knows whether there is such TIPs.
Therefore, let's collect the IME names which inserted CRLFs via TSF directly.
Differential Revision: https://phabricator.services.mozilla.com/D193879
I still don't understand what causes a recursive call while `TSFTextStore`
initializes content or selection cache. According to the crash rate, we
can stop doing this before the next merge.
Differential Revision: https://phabricator.services.mozilla.com/D182639
In some cases, focus notification is not correctly sent to widget. Therefore,
creating `TextStore` instance may cause memory leak because blur notification
is never sent for destroying the instance. Just for preventing memory leak
in browser-chrome tests, we need to avoid to create it without focus
notification.
Differential Revision: https://phabricator.services.mozilla.com/D171197
Some users may keep disabling TSF while we were improving TSF implementation due
to avoiding some bugs.
Currently, we don't test IMM handler. Therefore, I think that we should rename
the pref and make users run well-tested path to handle IME.
Depends on D159818
Differential Revision: https://phabricator.services.mozilla.com/D159819
Masayuki Nakano