2014年2月22日
Bug-org 501496の修正によるregressionです。印刷プレビューの画面でEscキーを押しても終了しなくなっていました。
印刷プレビューのUI側では、keypress
イベントでハンドリングされていましたが、nsPrintPreviewListener
がWebコンテンツやGecko自身にハンドリングされないように、keydown
イベントでpreventDefault()
を呼び出していたため、keypress
イベントが発生しなくなっていたのが原因でした。
UI側で、keydown
イベントを処理するように変更して修正しています。
Bug-org 501496の修正によるregressionです。クラッシュ後に表示される、タブやウインドウの復元画面でキーボードで、ツリー(リストっぽく見えますが)を操作できないというバグです。
復元画面では、全体が↑、↓、PageUp、PageDown、Home、Endでスクロールしないようにこれらのkeydown
イベントでpreventDefault()
を呼び出していました。そのため、XULの<tree>
要素がkeypress
イベントでハンドリングできなくなっていました。
XULの<tree>
要素は、keydown
イベントをハンドリングするように修正し、復元画面のスクロール抑制のためのコードを削除して、スクロール機能も利用できるようにしています。
WidgetTouchEvent
だけ、プライベートデータをコピーする際に、なぜかnsIWidget
への参照もコピーしていたというバグです。
書いた本人に確認したところ、意図的にそうした訳ではない(単にデフォルトコンストラクタに頼っただけ?)、という回答が来たので、一応、他と同様、コピーしないように修正しました。
JS側でタッチイベントを保存しておいて、ウインドウを閉じても、そのウインドウのインスタンスが保持されてるため、メモリリークが発生していた可能性もゼロではないのですが、まあ、狙ってやらない限り無理なので、実際には問題は起きてなかったと思います。
全イベントのコンストラクタをできるだけ同じ形にして、各メンバは明示的に初期化するようにしよう、というバグの、InternalTransitionEvent
版です。
InternalTransitionEvent(bool aIsTrusted, uint32_t aMessage,
const nsAString& aPropertyName,
float aElapsedTime,
const nsAString& aPseudoElement);
を、
InternalTransitionEvent(bool aIsTrusted, uint32_t aMessage);
に変更しています。これ、何が問題化というと、次のようなコードを書いた時、何が何だか分からないんですよね。
InternalTransitionEvent event(false, 0, EmptyString(), 0.0f, EmptyString());
メンバに直接代入するようにすれば、無駄を省けるし、何に対して値を設定しているのか明瞭になるので、コードが読みやすくなり、イージーミスも減るという訳です。
全イベントのコンストラクタをできるだけ同じ形にして、各メンバは明示的に初期化するようにしよう、というバグの、InternalAnimationEvent
版です。
InternalAnimationEvent(bool aIsTrusted, uint32_t aMessage,
const nsAString& aAnimationName,
float aElapsedTime,
const nsAString& aPseudoElement);
を、
InternalAnimationEvent(bool aIsTrusted, uint32_t aMessage);
に変更しています。
全イベントのコンストラクタをできるだけ同じ形にして、各メンバは明示的に初期化するようにしよう、というバグの、WidgetPointerEvent
版です。が、複雑なメンバ初期化を行うコンストラクタを利用してるコードが既にどこにもなかったので、複雑なものを削除しただけです。
今まではIME関係のイベントの配信状態を管理しているだけだったmozilla::TextComposition
クラスをnsEditor
で利用して、より、シンプルな設計にしようというバグです。
この修正によって、nsEditor
内部でのみ管理していた未確定文字列の状態等の一部が、mozilla::TextComposition
側で保存されるようになり、エディタ以外からもnsIWidget
やnsPresContext
からインスタンスを取得して参照可能になっています。
一方、nsEditor
は、compositionstart
イベントを受け取った場合にmozilla::TextComposition
をstrong referenceで保存し、compositionend
イベントを受け取った際に、解放するようにしています。そしてこの間は、mozilla::TextComposition
側にもハンドリング中のエディタへのweak referenceが保存されているので、相互参照可能になっています。
Geckoには開発初期段階から、VK_RETURN
というキーコードと、VK_ENTER
というキーコードの二つがあり、コア開発者、UI開発者共にその違いに悩まされてました。実際には、VK_ENTER
はGonkが間違ったマッピングを行っていた以外には使われていないものなので、削除することにしました。
この修正により、VK_ENTER
のハンドラは削除され、テストでVK_ENTER
を送信していた無意味なテストは、VK_RETURN
を用いるように書きなおされ、実際のユーザの操作で走るコードパスがテストされるようになっています。
リスキーですが、Javascriptから見た場合に、KeyboardEvent.DOM_VK_ENTER
がundefined
になっています。これによって、これを参照していた条件が、KeyboardEvent.keyCode
がゼロの場合に一致してバグを産まないかという点が懸念されますが、他のブラウザではKeyboardEvent.DOM_VK_*
は定義されていないので、問題があるとしたら、アドオンのみだと思います。
もし、それ以外に何か問題を見つけたら、早めにバグ報告をお願いします。影響が大きい場合は、KeyboardEvent.DOM_VK_ENTER
だけを元に戻せば互換性を維持しつつ、修正も活かすことができるので。
全イベントのコンストラクタをできるだけ同じ形にして、各メンバは明示的に初期化するようにしよう、というバグの、InternalUIEvent
版です。
InternalUIEvent(bool aIsTrusted, uint32_t aMessage, int32_t aDetail);
を、
InternalUIEvent(bool aIsTrusted, uint32_t aMessage);
に変更しています。
しかし、この修正で思いましたが、Event.detail
値がゼロ以外になることってほとんどないんですね。
全イベントのコンストラクタをできるだけ同じ形にして、各メンバは明示的に初期化するようにしよう、というバグの、WidgetSimpleGestureEvent
版です。
WidgetSimpleGestureEvent(bool aIsTrusted, uint32_t aMessage, nsIWidget* aWidget,
uint32_t aDirection,
double aDelta);
を、
WidgetSimpleGestureEvent(bool aIsTrusted, uint32_t aMessage, nsIWidget* aWidget);
に変更しています。
これは、uint32_t
とdouble
が並んでるので、変数を渡してない限り、本当に意味不明な初期化になっていました。
nsIWidget::NotifyIME()
は、通知内容を数字ひとつ渡しているだけだったので、nsIWidget::NotifyIMEOfTextChange()
だけが特別扱いになっているのが以前から気になっていました。
この修正で、数値ではなく、構造体を渡すようにしたので、今後、簡単に追加情報を含めて、IME関係の通知後にある典型的な処理を省いたりすることで、単純化・ランタイムコストの削減を行いたいと思っています。引数は以下のIMENotification
構造体のみになります。
enum IMEMessage MOZ_ENUM_TYPE(int8_t)
{
// XXX We should replace NOTIFY_IME_OF_CURSOR_POS_CHANGED with
// NOTIFY_IME_OF_SELECTION_CHANGE later.
NOTIFY_IME_OF_CURSOR_POS_CHANGED,
// An editable content is getting focus
NOTIFY_IME_OF_FOCUS,
// An editable content is losing focus
NOTIFY_IME_OF_BLUR,
// Selection in the focused editable content is changed
NOTIFY_IME_OF_SELECTION_CHANGE,
// Text in the focused editable content is changed
NOTIFY_IME_OF_TEXT_CHANGE,
// Composition string has been updated
NOTIFY_IME_OF_COMPOSITION_UPDATE,
// Request to commit current composition to IME
// (some platforms may not support)
REQUEST_TO_COMMIT_COMPOSITION,
// Request to cancel current composition to IME
// (some platforms may not support)
REQUEST_TO_CANCEL_COMPOSITION
};
struct IMENotification
{
IMENotification(IMEMessage aMessage)
: mMessage(aMessage)
{
switch (aMessage) {
case NOTIFY_IME_OF_TEXT_CHANGE:
mTextChangeData.mStartOffset = 0;
mTextChangeData.mOldEndOffset = 0;
mTextChangeData.mNewEndOffset = 0;
break;
default:
break;
}
}
IMEMessage mMessage;
union
{
// NOTIFY_IME_OF_TEXT_CHANGE specific data
struct
{
uint32_t mStartOffset;
uint32_t mOldEndOffset;
uint32_t mNewEndOffset;
uint32_t OldLength() const { return mOldEndOffset - mStartOffset; }
uint32_t NewLength() const { return mNewEndOffset - mStartOffset; }
int32_t AdditionalLength() const
{
return static_cast<int32_t>(mNewEndOffset - mOldEndOffset);
}
bool IsInInt32Range() const
{
return mStartOffset <= INT32_MAX &&
mOldEndOffset <= INT32_MAX &&
mNewEndOffset <= INT32_MAX;
}
} mTextChangeData;
};
private:
IMENotification();
};