2015年9月17日
TSFで、Win8.1やWin10に付属しているMS製のTIP、MS Pinyin、MS Wubi、MS Changjie、MS Quickでは、未確定文字列を入力しても、選択文字列が置換されず、選択範囲の先頭に未確定文字列が挿入されるというバグです。
TSFTextStoreのログを見ると、これらのTIPでは未確定文字列の入力開始時にITextStoreACP::QueryInsert()が呼び出され、その返り値が未確定文字列が置き換える範囲となってITfContextOwnerCompositionSink::OnStartComposition()が呼び出されていました(元に、QueryInsert()の返り値を選択範囲そのままで返すと正しく置換されました)。
GeckoのQueryInsert()の戻り値もちょっとおかしいのですが、その値が未確定文字列の置き換え範囲として利用されるのはこのケースでは明らかにおかしいので、TIPから利用しているカウンターパートを探してみたところ、ITfRange::AdjustForInsert()の呼び出し時に呼び出されてそうです。このドキュメントによると、
This method should be used to prepare a range to initiate a new composition, before editing begins. It should be used only when the text is not inserted at the current selection.
とあります。そこで、中国語のTIPのバグだと断定し、ハックを入れることで対処することにしました。
入れたハックはややリスキーなもので、QueryInsert()の戻り値をこれらのTIPがアクティブな時に書き換えるというものです。本当はOnStartComposition()の方で読み替えて対応したかったんですが、それを行うと、続いて呼び出される様々なメソッド全てにハックが必要で不可能に近いことに思えたので、このようにしています。
TSF対応最初のバージョンである41にも修正を入れています。
twitter.comでリプライを入力する時にATOKのナビバーや、様々なTIPのサジェストウインドウや、候補ウインドウがキャレット位置等の適当な位置に表示されないというバグです。
調査してみると、ContentCacheInChildがNOTIFY_IME_OF_POSITION_CHANGEを受け取ってWidgetQueryContentEventを利用してコンテンツの情報を取得し、キャッシュする際に選択範囲がおかしな値になっていることがあることが分かりました。その原因は今でも分かっていませんが、NOTIFY_IME_OF_POSITION_CHANGEを通知してはいけないタイミングが未だに存在し、ContentEventHandlerがフォーカスを持ったcontenteditableなエディタだけではなく、ドキュメント全体をターゲットにしてコンテンツを取得して値がおかしくなっているようでした。
根本的な原因の調査にはものすごく時間がかかることが明らかなので、無理の無いアプローチでバグを隠してしまうことにしました。まず、WidgetQueryContentEventをContentEventHandlerに渡す場所を変更しました。エディタがフォーカスを持たない場合は今まで通りEventStateManager::PreHandleEvent()から処理しますが、IMEContentObserverのインスタンスが存在する場合はIMEContentObserver経由で呼び出すようにしました。
これにより、WidgetQueryContentEventの戻り値の値をキャッシュし、適切な時に破棄することが可能になります。今回の場合、選択範囲を正しく返すことができればひとまずバグを隠すことができるので、NS_QUERY_SELECTED_TEXTに対しては常にキャッシュしてある情報を返すようにしました。これにより、安全なタイミングでキャッシュされた正しい選択範囲の情報が常にTIPに渡るようになっています。
なお、このバグの調査のために書いたIMECntentObserverのロギングコードはそのまま投入されていますので、NSPR_LOG_MODULES=IMEContentObserver:5でIME (widget)への通知の発行部分をログから調査できるようになっています。
また、IMEContentObserver::EditAction()が適切に呼び出されないバグも発見したため、修正していますが、これの副作用により、inputイベントの発火仕様が若干変更されています。未確定文字列が空になった時にはinputイベントが発火されていませんでした。これは、未確定文字列になった時には直後にcompositionendが来て、その後にinputイベントが確実に発火されるという前提に基づいたものでしたが、compositionendを伴わないケースではinputイベントが発火するようになっています。この変更は実際にはどのIMEを利用していてもおそらく実際には発生しない差異だとは思います(知る限り、そのような動作をしたIMEを知らない)。
内部処理の変更です。IMEContentObserverは複数の選択範囲変更の通知が重なった場合、最後のものだけを通知するようにしていますが(それ以前のものはIMEからすると上書き済みの選択範囲なので不要な変化)、その理由(未確定文字列の変化で発生したのか否か、選択範囲をIMEからの指定で変更したために発生したのか否か)は最新の理由ではなく、全ての変更理由がそれに該当する場合にのみtrueとしていました。
しかし、最新の選択範囲を通知する際に古い理由を添えるのはおかしな話で、他の理由により選択範囲が変更されていたことを知る必要があるなら、それはそれで通知すべきです。しかも、ContentCacheInParentではこのようなことをしていないため、結局、e10sモードでは古い理由は破棄される可能性がありましたので無意味でした。
今回の修正により、IMEContentObserverとIMEContentObserver::SelectionChangeEventの実装が簡略化されています。
大きな変更になるので、予め記事を書いておきましたが、その修正がようやく完了しました。その結果、
WidgetEvent::messageはWidgetEvent::mMessageに改名WidgetEvent::mMessageは名前付きのenumである、mozilla::EventMessageになったEventMessageを格納している変数名は、基本的にはmessageかmsgを含むものとなったEventMessageの各項目の名前は、eプリフィックス付きのできるかぎり短い名前になった
- DOMイベントの元となるイベントの場合、その名前を利用するようになった
- 名前があまりにも一般的すぎる場合は
eの直後にイベントのグループ名等を含むようになった
- 標準仕様策定前に使われていたイベントや、現在廃止予定のイベントは
eLegacyで始まる名前になった
という形になっています。内部の名前とDOMイベントの名前のズレや、歴史的経緯を知らない開発者向けにレガシーイベントであることを名前で示せるようになったのは大きかったです。
また、VisualStudio、gcc、lldb、Xcode全てで変数の内容を見ると数値ではなく、enumで付けられている名前が表示されますのでデバッグしやすくなっています。
ちなみに安全のため、ひとつの置換をひとつのパッチで行ったため、最終的には270個のパッチに分割され、関係者からも単独のバグではさすがに過去最多のパッチ数じゃないかと言われています。
Bug-org 895274の修正中に、内部イベント用であるEventMessageがinteger型で指定してイベントを発火できてしまうscriptableなnsIDragService::FireDragEventAtSource()というのを発見しました。
このような内部処理の変更で破綻するAPIは最悪なので変更か隠してしまうしかありません。調べてみたところ、addons.mozilla.orgに登録されているアドオンでこれを利用しているものはありませんでしたし、Firefoxでも利用していませんでしたので、non-scriptableなメソッドに変更して解決しています。
ただの数値だったイベントメッセージは、Bug-org 895274の修正により、デバッガで数値から意味を調べるという作業・必要性が無くなりました。そこで、このバグでは、enumの定義の際に整数値を指定するのをやめ、0からの連続した値を利用するように修正しました。
また、今回の修正により、EventForwards.hに、const char* ToChar(EventMessage aEventMessage)と、const char* ToChar(EventClassID aEventClassID)を実装したので、MOZ_LOG()を利用する時や、printf()デバッグで簡単にイベントメッセージの名前を取得することができるようになっています。
2015年9月29日
Bug-org 1204523でMS-IME向けに修正したのと同じ症状がMicrosoft Office IME 2010でも候補ウインドウの表示時に発生しているというバグです。
原因はやはりおなじみのTS_E_NOLAYOUT絡みのバグでしたので、MS-IMEと同じハックを適用するようにしています。なお、今後新しいバージョンがリリースされることはなさそうなのでハックを無効にする設定は今回は用意していません。
また、失念していましたが、Microsoft Office IME 2007が存在し、これのテストが行えていませんでした。そこで、急遽製品を取り寄せたので、時間が取れ次第、こちらの動作も確認したいと思っています。
TSFのTS_E_NOLAYOUTのような問題がMac OS X 10.10.xのIMEでも発生しているというバグです。また、直前に確定した未確定文字列が新しい未確定文字列の変換候補に出てくるというバグも原因が同じバグだったのでこの修正により解決しています。
前回、Gecko Insiderに参加したときに元々このバグを教えてくれたさねゆきさんから再現方法を実演してもらった所、高速入力時にのみ時々発生するということが分かりました。そのことを踏まえてログをとってみたところ、やはり、IMEが想定しているよりも古いコンテンツを返してしまった場合にこのバグが発生していることが分かりました。
本当であればbug-org 1173694を修正し、WidgetQueryContentEventが未処理の未確定文字列の編集結果も含めた結果を返してくるようにすべきなのですが、43のサイクルが終了間際だったため、TSFTextStoreと同様にIMEInputHandlerに最新の未確定文字列の情報をキャッシュし、WidgetQueryContentEventを利用せずにそれを利用するようにして回避しました。
IMEContentObserverのログを記録できるようにしてみたところ、異常な回数、NOTIFY_IME_OF_POSITION_CHANGEを送信しているのを発見しました。もし無駄な呼び出しがあった場合、e10sモードではかなり無駄にCPUを走らせていることになります。
調べて見ると、nsRunnableの派生クラスを利用して、script blockerが取り除かれた瞬間に何らかの通知を送信すると、特にe10sモードではContentCacheが必ず、WidgetQueryContentEventを発火してきます。これを処理するためにContentEventHandlerがペンディングとなっているレイアウトをフラッシュすることで、reflowが発生し、これがNOTIFY_IME_OF_POSITION_CHANGEを送信の原因になっていることが多い事が分かりました。しかし、よく考えてみれば、何らかの通知を送信中に発生したreflowに対して、NOTIFY_IME_OF_POSITION_CHANGEを送信する必要はないのです。なぜなら、そのreflowの結果から計算された情報をWidgetQueryContentEventで返すため、IMEからすると既に最新のレイアウト情報を持っているのに余分なNOTIFY_IME_OF_POSITION_CHANGEを受け取ることになります。
これを解決するには通知を送信中に追加で発生した変更情報はマージし続けたり、不要なものは無視する必要があります。しかし、今までのIMEContentObserverは各通知を個別のnsRunnableの派生クラスに保存し、新しい通知が来た場合にはまだ前回の通知が処理されていなくても、再びnsRunnableの派生クラスを生成していました。そのため、この複雑な状態を管理するためには多くのフラグを必要とします。しかし、そのようなコードでは限界が見えていますので大きく設計を変更することにしました。
まず、通知内容は全て実際に送信されるまでIMEContentObserver自身が保持し続けるようにしました。これにより、nsRunnable派生クラスを生成した後、実行までに新しい通知が発生しても、変更内容をマージするだけで済みます。
次に、nsRunnableクラスを各通知ごとに生成するのではなく、通知を送信するクラスを一つにまとめ、これが実行された時に所定の順番で通知が行われるようになりました。これにより、通知の順番を常に保証することが可能になりました(テキストの変更範囲通知、選択位置の変更通知、レイアウトの変更通知の順でなくてはIMEから見た場合に矛盾が発生するため)。
最後に、順番の保証にも絡みますが、再帰呼び出しが発生していないか確認するようにし、再帰呼び出しになってしまう場合には実行予約だけをメインスレッドに投げるようにしました。これにより、スタックオーバーフローが発生する可能性も修正しています。