1467373 - [TSF][TS_E_NOLAYOUT] Separate TS_E_NOLAYOUT hacks in TSFTextStore::GetTextExt() into new methods and disable these hacks on Win10 Build 17643 or newer

Status: RESOLVED FIXED

(Core :: Widget: Win32, enhancement)

Description

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

Now, we have a lot of hacks of TS_E_NOLAYOUT bug of Windows:
https://searchfox.org/mozilla-central/rev/cf464eabfeba64e866c1fa36b9fefd674dca9c51/widget/windows/TSFTextStore.cpp#4428-4625

These hacks are ugly and makes difficult to check entire code in GetTextExt(). So, let's move those hacks into new methods, and then, stop running almost all of these hacks on Win10 Build 17643 or later since the bug of Windows has been fixed!

See also my blog post: https://d-toybox.com/studio/weblog/show.php?id=2018060200

FYI: Stopping running the hacks will make some IME behavior better.

Comment 1

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96994f169295ee2a9c1d7412db5e96536b679b57

Comment 2

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

Attachment: Bug 1467373 - part 1: Split hack for TS_E_NOLAYOUT part from TSFTextStore::GetTextExt()

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.

Review commit: https://reviewboard.mozilla.org/r/260948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260948/

Comment 3

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

Attachment: Bug 1467373 - part 2: Skip hacks for most TIPs in TSFTextStore::HackNoErrorLayoutBugs() if running on Windows 10 build 17643 or newer

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.

Review commit: https://reviewboard.mozilla.org/r/260950/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260950/

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 8996972 [details]
Bug 1467373 - part 1: Split hack for TS_E_NOLAYOUT part from TSFTextStore::GetTextExt()

https://reviewboard.mozilla.org/r/260948/#review268206

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8996973 [details]
Bug 1467373 - part 2: Skip hacks for most TIPs in TSFTextStore::HackNoErrorLayoutBugs() if running on Windows 10 build 17643 or newer

https://reviewboard.mozilla.org/r/260950/#review268210

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79e964194d99
part 1: Split hack for TS_E_NOLAYOUT part from TSFTextStore::GetTextExt() r=m_kato
https://hg.mozilla.org/integration/autoland/rev/845a7347f1a9
part 2: Skip hacks for most TIPs in TSFTextStore::HackNoErrorLayoutBugs() if running on Windows 10 build 17643 or newer r=m_kato

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/79e964194d99
https://hg.mozilla.org/mozilla-central/rev/845a7347f1a9