If WM_GETOBJECT for OBJID_CARET is received but a11y module is not active,
IME module should create native caret over our caret because Windows will
handle the request with native caret automatically and we don't need to
enable a11y module only for it.
This patch makes IMEHandler store whether such message has been received and
makes TSFTextStore create native caret when composition, selection or layout
is changed because especially when there is composition, only TSFTextStore
knows correct position to put caret if there is composition or some dispatched
events have not been handled by content process yet.
Note that IMMHandler already does that since some legacy IMEs require native
caret to show its UI and we cannot check active IME strictly. Therefore, this
patch does not touch IMMHandler.
Differential Revision: https://phabricator.services.mozilla.com/D13961
--HG--
extra : moz-landing-system : lando
IMEHandler needs to create native caret later (when there is no composition).
Therefore, IMEHandler should manage whether it creates native caret or not
and IMMHandler and TSFTextStore should create/destroy native caret via
IMEHandler.
Note that this patch makes IMMHandler stops managing whether native caret
is created for plugin or not because native caret is created only one instance
and anyway IME handlers should stop managing native caret when they loses
focus.
Differential Revision: https://phabricator.services.mozilla.com/D13960
--HG--
extra : moz-landing-system : lando
If a11y module is active, it observers caret position and size, and when caret
position or size is changed, it creates/moves native caret to overlap with
our caret.
On the other hand, IME module also creates native caret if active IME requires
it. Therefore, both of them conflicts each other.
This patch makes IME module stop touching native caret if a11y module is active.
Although, a11y module with Flush Player does not work well for IME. Therefore,
this patch keeps the conflict between them as-is for now.
Differential Revision: https://phabricator.services.mozilla.com/D13959
--HG--
extra : moz-landing-system : lando
If TSFTextStore fails to get selection, e.g., the content is really odd like
fuzzing tests, its mSelectionForTSF is marked as dirty. However, Windows may
try to retrieve writing mode while we're creating new TSFTextStore. Then, we
may hit MOZ_ASSERT(!mDirty) in TSFTextStore::Selection::GetWritingMode() in
debug build.
So, we need to make some callers of GetWritingMode() check whether selection
is marked as dirty.
Differential Revision: https://phabricator.services.mozilla.com/D14137
--HG--
extra : moz-landing-system : lando
ATOK 2016 and newer may show candidate window at odd position temporarily
when user converts a word quickly (e.g., keep pressing space bar).
For avoiding this flicker, we should keep hacking GetTextExt() result for
ATOK 2016 and later even after Windows fixes TS_E_NOLAYOUT bug.
We should get feedback from each CJKT testers at least one cycle after Win10
RS5 is released. Until then, we should not stop hacking GetTextExt() result
in late Beta nor Release builds.
Microsoft Pinyin and Microsoft Wubi (Simplified Chinese TIPs) work better on
Windows 10 Build 17643 or later (i.e., TS_E_NOLAYOUT bug is fixed). However,
they sometimes do not show candidate window, perhaps, the reason is something
stateful bug in them. Therefore, we still need to hack the result of
GetTextExt() until they fix this bug.
Currently, TSFTextStore::MaybeHackNoErrorLayoutBugs() checks pref to enable
hack first, then, check if active TIP is the target of pref. This was intended
to save comparison cost of GUIDs. However, we don't need to worry about the
cost and that was not makes sense since all prefs are true by default.
So, this patch makes the big if-elseif blocks with switch-case with
TSFStaticSink::ActiveTIP(). Then, each case block starts to check if
- if Windows still TS_E_NOLAYOUT bug of GetTextExt().
- if corresponding pref is true.
Note that this duplicates some code for making the code look easier.
E.g., eMicrosoftOfficeIME2010ForJapanese case is duplicated from
the eMicrosoftIMEForJapanese case. eMicrosoftPinyin and eMicrosoftWubi case
is duplicated from the eMicrosoftChangJie and eMicrosoftQuick case.
As we know, GUID comparison is not cheap if it's required a lot. Unfortunately,
we need to check it more in TSFTextStore::MaybeHackNoErrorLayoutBugs() and
it's called a lot. So, even though mapping from GUID to TIP is expensive,
we should do it only once. Note that most users won't change IME during a
browser session, so, running this expensive method once must be reasonable.
On the other hand, we don't allow to make damage to start up performance,
we should avoid to do it as far as possible. For example, when we need to
check if active TIP is a specific TIP, we should check current language.
At Windows 10 build 17643, Microsoft fixed the bug of TSF which returns E_FAIL
to TIP when GetTextExt() returns TS_E_NOLAYOUT. With this fix, most TIPs do
not have any problems even if we return TS_E_NOLAYOUT. So, unless active
TIP still needs the hack, the method can skip the hack if running on build
17643 or later.
Note that we still need to support Japanist 10 and Microsoft Office IME 2010.
It confirmed that Japanist 10 has a bug of handling TS_E_NOLAYOUT. On the
other hand, we have not tested Microsoft Office IME 2010 since it's installable
only into Win7 or Win8 and needs to upgrade it to Win10 for testing, but I
do not have the license. After the fix comes into release channel, I'll be
able to test it though (my main environment is Win10 and it was installed
before upgraded). So, we need to be back after Microsoft releases the fix.
MozReview-Commit-ID: 2BzkDvHTKyI
--HG--
extra : rebase_source : ee0261c83c5a1ab7b2aa2a8f476f0c6634e2cf34
The block in TSFTextStore::GetTextExt() which decides whether we should return
S_OK with unmodified character rectangle rather than TS_E_NOLAYOUT is too big.
Additionally, we need to add new condition to check Windows 10's version there.
That makes the large block more complicated. So, we should split the block
off from TSFTextStore::GetTextExt(). Then, we can use early-return-style to
reduce the deep indentations.
MozReview-Commit-ID: J2BJMB1QD0T
--HG--
extra : rebase_source : 3c86b5ed3a83fda1045a6453250e784f11419b97
When user removes all composition string of MS Pinyin, MS Wubi, MS ChangJie and
MS Quick with Backspace key, IME commits last character temporarily and
restart composition to replace the last character with empty string when
user tries to remove last one character.
This causes flicking composition string because the additional composition
selects the character and it may be painted immediately before removed, and
also editor will have unnecessary undo transaction.
Therefore, as same as bug 1208043, TSFTextStore::RecordCompositionStartAction()
should restart last composition in such case. Fortunately, we implemented
similar code for bug 1208043, however, unfortunately, we don't have preceding
pending compositionstart in this case. Therefore, this patch makes
pending compositionend store start offset of composition. Then, we can
restart composition only with information stored by pending compositionend
action. Additionally, this patch renames the method checking pending
actions for self-describing the new meaning.
MozReview-Commit-ID: 1RyuacxEbky
--HG--
extra : rebase_source : 1c8ecc0b63114ae65c77cd76cb85a21d2716442c
We always struggle with a lot of IME bugs on Windows. Currently, any IME
vendors should've already released TIP for TSF rather than legacy IMM-IME
since IMM-IME is not available on UWP apps. Additionally, due to API
limitation, it's difficult to get human-friendly name of IMM-IME. So, let's
collect only TIP names of TSF on Windows. This must be enough.
Note that we cannot get common-English name even though the API to retrieve
TIP name taking language code. Therefore, a TIP may be collected with
different name, e.g., one is Japanese name and the other is English name.
If we collect GUIDs of TIP, we can avoid this issue. However, it's
difficult to collect both GUID and human-friendly name since Telemetry
key is up to 72 characters.
Currently, I give up to avoid this duplicated issue. Perhaps, this is not
so serious issue since most TIP users must match language of TIP and their
system language settings. Therefore, this patch collects Locale ID of
TIP and description of it. Locale ID is necessary because some TIPs may be
named same name for different languages. For example, both Japanese and
Hangul IMEs of Microsoft are named as "Microsoft IME".
MozReview-Commit-ID: IeSxfeqS62a
--HG--
extra : rebase_source : b269ce3c41f7a998193972afb31183a61d3948be
Currently, TSFTextStore::GetTextExt() won't return TS_E_NOLAYOUT error when
ATOK retrieves text rect of all of the composition string.
However, if user converts 2nd or later clause, ATOK retrieves text rect after
start of the character of selected clause. Returning TS_E_NOLAYOUT in this
case causes candidate window being positioned temporarily below first character
of the composition string.
For avoiding the flicker of the candidate window, TSFTextStore::GetTextExt()
shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the
composition string.
MozReview-Commit-ID: Cp17HmP2QGK
--HG--
extra : rebase_source : bdd2d2867cccdcf1fe39612ca2f52872d472cb7a
Old ATOK referred native caret position to decide its candidate window position.
However, at least ATOK 2016 does not need to refer it. Additionally, if we
create native caret for ATOK 2016, the candidate window position, ATOK 2016
refers the native caret only when we cannot return expected rect. Therefore,
only immediately after modifying composition string, the position is different
from actual position by a couple of pixels and that looks like flicks the
candidate window.
So, we should stop creating native caret for ATOK 2016 (as same as ATOK 2017).
MozReview-Commit-ID: LsmVXCmRIzc
--HG--
extra : rebase_source : 30b7d15cb23e567b14e1231909aa5a17bdf909a6
TSFTextStore should discard pending composition update actions when it records
end composition update action because end composition update action causes
dispatching eCompositionCommit event and it replaces old composition string
anyway. So, following eCompositionChange which is dispatched by preceding
composition update actions are just redundant.
MozReview-Commit-ID: HBHx2jA15ro
--HG--
extra : rebase_source : 74d1e91d73bf9c8182a9c5e3fd55d052d8ec4bea
OnUpdateComposition() may be called without new range instance by some TIPs
when they starts to modify composition string. At this timing, TSFTextStore
should append a pending action for dispatching keyboard event into the queue.
Without this patch, OnUpdateComposition() creates incomplete pending action
for composition update and then, MaybeDispatchKeyboardEventAsProcessedByIME()
appends pending action for dispatching keyboard event from another point
immediately (e.g., from SetText()), then, finally, the caller of
MaybeDispatchKeyboardEventAsProcessedByIME() appends another pending action
for composition update with proper composition string. Therefore, the
first pending action for composition update clears composition string before
actually updating it with new composition string. In other words, new
pending action for dispatching keyboard event splits a pending composition
update. So, this patch prevents the splitting.
MozReview-Commit-ID: 9pYO9pm3Vh9
--HG--
extra : rebase_source : a8fb903174c0f1af61a71175dbbf01b7b1c5cb53
TSF doesn't send WM_KEYDOWN nor WM_KEYUP to us while it handles a key message
with ITfKeystrokeMgr::KeyDown() or ITfKeystrokeMgr::KeyUp(). Therefore,
TSFTextStore needs to store handling key event message during calling
those methods and if it does something, we need to dispatch eKeyDown event
or eKeyUp event before dispatching any events.
However, we shouldn't dispatch WidgetKeyboardEvent during a document lock
because TSF/TIP do not assume that document is broken during a document lock.
Therefore, TSFTextStore needs to put it as a pending action into the queue.
So, this patch wraps this with
TSFTextStore::MaybeDispatchKeyboardEventAsProcessedByIME(). It checks if
there is a document lock when it's called. If it's locked (and not yet
dispatched keyboard event for the handling key message), it adds pending
action to dispatch keyboard event later. Otherwise, (and not yet dispatched
one), it dispatches keyboard event directly.
MozReview-Commit-ID: 9rJTJykVLyf
--HG--
extra : rebase_source : 4f8297b2b9fe2905e4cd1f64086fcdbe3d0b6035
Although we haven't any bug reports caused by this, this is a really old bug.
When we implement TSFTextStore, we decided to use queue of dispatching
events and flush it when document lock is unlocked. When we implement the
queue, we got this regression.
When TSFTextStore::SetText() is called with different range from current
selection range, TSFTextStore::SetSelectionInternal() add
PendingAction::SET_SELECTION into the queue first for replacing existing
text or inserting text into different position if there is no composition.
Then, TSFTextStore::InsertTextAtSelectionInternal() inserts text at the new
selection range.
When TSFTextStore::FlushPendingActions() is called after that, eSetSelection
should be dispatched and then, new text is inserted wit a set of composition
events. However, we forgot to dispatch creating eSetSelection event.
So, this patch just dispatches the event.
MozReview-Commit-ID: Hw8FTB1R5kR
--HG--
extra : rebase_source : 8a119f1f48b167d9423fc89ce0efc722313c3732
The renaming problem is, when I try to convert 2nd or later clause of
composition string with Japanist 10, it shows candidate window below the
start of composition string first, then, it moves candidate window to
below the selected clause. This is caused by our bug of the hack in
TSFTextStore::GetTextExt().
First, we compute wrong minimum modified
offset of mContentForTSF. It stores last composition string when it's
initialized. Then, when a part of composition string is modified, it
sets minimum modified offset with the last composition string. However,
we don't update it when we receive notifications from content which means
all dispatched composition events are handled in content and
ContentCacheInParent stores character rects at least in this time. So,
this patch adds TSFTextStore::Content::OnCompositionEventsHandled() to
update the last composition string.
Next, TSFTextStore::GetTextExt() always adjusts acpStart to start of
composition string when acpStart is larger than composition start.
However, this causes this remaining problem. If ContentCacheInParent
stores character rects of even older composition string, we should use
it as far as possible. This must not be problem in most cases since
most Chinese characters and Japanese Kana characters have same width.
This touches share code of the hack between any TIPs. However, this must
not be risky because this patch just reduces amount of adjusting acpStart
offset in safe range.
MozReview-Commit-ID: KlDeaGa26UG
--HG--
extra : rebase_source : 6d906f9810b8e067018f7ff3ab2fd31f5bef49f6
Similar to ATOK, Japanist 10 requests all or part of composition string.
If we return TS_E_NOLAYOUT in this case, you'll see candidate window at
top-left of the screen.
For avoiding this issue, we should not return TS_E_NOLAYOUT to Japanist 10
when the query range is in composition string.
MozReview-Commit-ID: 2OPafUO5PQC
--HG--
extra : rebase_source : bd7a594d8d3540374d61860651b69528aa6e3793
TIP name of ATOK Passport's ATOK 31.1.2 is just "ATOK", not "ATOK " + release
year. Therefore, TSFStaticSink::IsATOKActiveInternal() returns false when
new ATOK is active. This patch updates it.
Additionally, this patch adds GUID list of new Japanese TIPs with comment.
They should be useful when somebody needs to add hack for them.
MozReview-Commit-ID: 6L5SjWEK6i0
--HG--
extra : rebase_source : e3ac9eb1377ee9143a4e2e9fa2ce4be98983bb4b
Currently, TSFTextStore::GetTextExt() refers mComposition for doing its own
hack. However, this means that it refers composition in TIP. However,
query event is computed with content information. So, even if TSFTextStore
dispatched eCompositionCommit event, it may not be handled by content yet.
In this case, we need information relative to last composition string.
So, TSFTextStore::GetTextExt() should refer IsHandlingComposition() and
last composition string information stored by mContentForTSF.
MozReview-Commit-ID: KMqrDmnUldU
--HG--
extra : rebase_source : cea97c9e9117028913cf45e4dc1e83e2b4f102e9
If remote process hasn't handled dispatched commit event yet, TSFTextStore
needs to dispatch query content event relative to latest composition string
information. So, TSFTextStore::mContentForTSF should cache composition start
and composition string length until pending composition events are handled
by content actually.
MozReview-Commit-ID: ARM851nNZGz
--HG--
extra : rebase_source : d4f1ddb197d3168f2db52849b2982779c388e2e8
When composition events are handled by content actually, widget receives
NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED notification. If focused content
is in a remote process, this is notified only when all sending composition
events are handled in the remote process. So, when widget receives the
notification can there is no composition in IME, that means that nobody is
composing composition at that time.
This patch adds TextEventDispatcher::IsHandlingComposition() which returns
false only when nobody has composition and makes TSFTextStore refer this
method because TSFTextStore needs to know if focused content has composition
in any cases.
MozReview-Commit-ID: F1ZZgFJAArD
--HG--
extra : rebase_source : 65e7f592e0ffd1c516e4dab16ab4ca8d7171f954
This patch adds following Microsoft's IMEs into the black list which set
their open state to "closed" when input scope is set to IS_URL and sets
input scope for the URL bar to IS_DEFAULT.
Additionally, this adds a new pref to disable this hack because a lot of
users will affect this hack but perhaps, somebody may not like this if
they use tablet.
The new black listed IMEs:
- Microsoft Bopomofo
- Microsoft ChangJie
- Microsoft Phonetic
- Microsoft Quick
- Microsoft New ChangJie
- Microsoft New Phonetic
- Microsoft New Quick
- Microsoft Pinyin
- Microsoft Pinyin New Experience Input Style
- Microsoft Wubi
- Microsoft IME for Korean (except on Win7)
- Microsoft Old Hangul
MozReview-Commit-ID: BwJKFcu80B8
--HG--
extra : rebase_source : 75aeed04504b476520102984ab6e7875c98b36c8
The functions changed are given signed arguments (that are converted to unsigned).
Changing them to signed resolves the warnings and preserves the original values.
MozReview-Commit-ID: BxIAECFiuQR
--HG--
extra : rebase_source : c48cef46f3ad5a060ad1f33d1c97744bfa8a82d7
When "mozAwesomebar" is set to inputmode value, that means that the Smart
Location Bar gets focus. In that case, we should notify IME of input scopes
as "URL" because on-screen keyboard for URL has some useful additional keys
but they are not hindrances even when users want to type non-URL text.
On the other hand, MS-IME for Japanese and Google Japanese Input changes their
open state to "closed" if we notify them of URL input scope. A lot of users
complain about this behavior. Therefore, we should notify only them of
"Default" input scope even when "mozAwesomebar" has focus.
MozReview-Commit-ID: DIgqpR7TXQx
This warning is saying the operation may be undefined because the value of
->left is not guarenteed to be the same because of undefined order of operations.
Fortunately, this seems like a typo and we actually meant to assign ->bottom
MozReview-Commit-ID: H5G8fnDwIJP
--HG--
extra : rebase_source : cd6a2463a90b6675b12bb8255fe605937771bee8
Currently, TSFTextStore::GetIMENotificationRequests() tries to return notification requests for focused text store. However, if there is sEnabledTextStore, all notifications will be sent for it. Therefore, TSFTextStore::GetIMENotificationRequests() should return notifications for sEnabledTextStore without checking focused text store in the thread manager.
For example, we try to keep active composition even if we're being inactivated because Windows steals focus from us when our main process becomes busy but user may want to continue to compose the composition after our process becomes available.
Additionally, if TSFTextStore::GetIMENotificationRequests() doesn't return IMENotificationRequests::NOTIFY_DURING_DEACTIVE, sEnabledTextStore will be broken by NOTIFY_IME_OF_BLUR which will be sent by IMEStateManager::OnChangeFocusInternal(). However, when we're being activated, NOTIFY_IME_OF_FOCUS won't be notified because focused remote process keeps having IMEContentObserver. Therefore, sEnabledTextStore won't be recreated and IME becomes unavailable.
Therefore, while sEnabledTextStore is not nullptr, TSFTextStore::GetIMENotificationRequests() needs all notifications even if it loses focus in the thread manager.
MozReview-Commit-ID: CC6gJIpHxUO
--HG--
extra : rebase_source : 02559fe8fea81b4247ef27043fc1d995981a9ba4
This is remaining cases of bug 1312302. TSF may set focus to context when it receives focus related message. In such case, TSF tries to retrieve selection but TSFTextStore::GetSelection() returns E_FAIL due to still not initialized, TSF crashes.
This patch moves the hack to TSFTextStore::GetSelection() and restrict to work only on problematic versions of Windows 10.
MozReview-Commit-ID: 6cTiZ4HCO18
--HG--
extra : rebase_source : 733377be55d52c43ef90d6e949cb851cf4c6dcb2
TSFTextStore::GetIMEOpenState() may be called a lot. Therefore, TSFTextStore should cache the compartment until shutting down.
MozReview-Commit-ID: 2jz8zQMBHRS
--HG--
extra : rebase_source : b322bcae129c73d7fdd1de080d525dbd6f34e6f8
sMessagePump is necessary only when WinUtils::(Get|Peek)Message() retrieves a message from the queue. Therefore, we can put off to initialize it until then.
MozReview-Commit-ID: ByMJk6AIw1r
--HG--
extra : rebase_source : e826fcd6e67b2b1599b31dda44140c1e7218da5a
sKeystrokeMgr is required only when WM_KEYDOWN or WM_KEYUP message are received. So, we can put off to initialize sKeystrokeMgr until then.
MozReview-Commit-ID: JsLeM0SYXG6
--HG--
extra : rebase_source : 766479689ec7739433b146fa5c0d7985fd6fc866
TIP name may be localed on some locales of Windows. Additionally, names may be updated in the future releases. So, it's safer to use GUID rather than name when TSFStaticSink checks active TIP is a specific one.
MozReview-Commit-ID: 6HNePZV7kgJ
--HG--
extra : rebase_source : 9d77c3124fc5ebeb82b9af1c13ae73633d9de4b8
Easy Changjei, a Traditional Chinese IME, isn't available on Firefox because:
* The vendor has gone and nobody keeps maintaining it.
* It crashes at first key press since it was built with older Visual Studio and depends on the version's CRT.
Therefore, we don't need to support it anymore.
MozReview-Commit-ID: LjyAvWsrlJ1
--HG--
extra : rebase_source : 481c4fdd5bbd6ed9984a101226f5232da9705430
Getting all prefs for TSFTextStore during initializing may make damage to start up performance.
So, each one should be retrieved when the one is actually necessary.
This patch creates TSFPrefs (I like better to name it TSFPreferences, but such long name isn't better when calling long name methods.) and implemented by simple macro.
MozReview-Commit-ID: A01LEAW4E7i
--HG--
extra : rebase_source : c471059c486c357eb270a7ea2ed1c5a07dd74e83
ITfProcessorProfiles are used by a debug method TSFTextStore::CurrentKeyboardLayoutHasIME() and TSFStaticSink (when it's initialized). However, TSFStaticSink isn't necessary until when TSFTextStore needs to hack something for specific IME or notifying IMEHandler of active TIP change. So, we can put off to create the instance of ITfInputProcessorProfiles and TSFStaticSink.
MozReview-Commit-ID: KcrqUbqz1do
--HG--
extra : rebase_source : f1821b782c6cd316a8f234a17ee3c92114547041
Creating them may be expensive due to allocating them in the heap. So, let's put off to create them when first use.
MozReview-Commit-ID: HDgijJo7brU
--HG--
extra : rebase_source : 9e4e68bd048185fe38fd98bef9b5711e86f68999
TIPs (and normal keyboard layouts) don't need IMC on focused window. So, in most environment, it's not necessary to restore default IMC of focused window.
Therefore, this patch makes IMEHandler not restore default IMC unless legacy IMM-IME is active and disassociate IMC from focused window when IMM-IME isn't active.
However, this is risky change. Therefore, the new behavior is disabled in default settings. On the other hand, we need the new behavior only when MS-IME for Japanese is active on Win10. Therefore, this patch adds a pref to enable/disable the hack and make it true in the default settings.
MozReview-Commit-ID: KAVxVT9CrsW
While initializing or destroying TSFTextStore, each object methods should be
called after the instance is grabbed by local variable since member variable
may be cleared by nested call to destroy a TSFTextStore instance.
MozReview-Commit-ID: CojLasqcDyB
Masayuki Nakano
Sylvestre Ledru
Makoto Kato
Xidorn Quan
Miko Mynttinen
Milan Sreckovic
Tom Ritter
Wes Kocher