Nov 30, 2007
APIを作る時はどんな使われ方をするかちゃんと考えてから作ろうという話
ツリー型タブでずっと前から発生条件が分からなくて困ってた「一つもタブが選択されていない状態」になってしまうバグの原因がやっと分かったので、速攻で修正した。
この問題は、以下の条件がすべて揃った時に発生していた。
- タブバーの末尾に、ツリー状になったタブがある
- そのツリーが折り畳まれた状態で、ツリーの最上位の親のタブが選択されている
- そのタブを閉じる操作を行う
この時、内部的にどういう処理が行われていたかというと、以下のような感じだ。
- removeTabメソッドが呼ばれる
- removeTabメソッドにてTabCloseイベントが発生
- TreeStyleTabBrowserのインスタンスがTabCloseイベントを捕捉
- 1で閉じられようとしているタブがツリーの親で且つ子孫タブが折り畳まれている場合、TreeStyleTabBrowserが子孫タブを先に閉じる
- TreeStyleTabBrowserのTabCloseイベントに対する処理が完了
- removeTabメソッドの残りの部分に制御が戻る
- 閉じようとしているタブが、選択されたタブだった場合、次に選択するべきタブを決定し、そのタブを選択する
- removeTabメソッドの処理が完了
この「TabCloseイベント」というのがFirefox 2以降で使えるAPIなんだけれども、問題は7のステップで起こっていた。7のステップで「次に選択するべきタブ」を決定するためのキーとして「現在開いているタブの総数」を使っているのだけれども、その値は1のステップの間に取得しているため、1から7までの間のどこかでタブの数が増減した場合、タブの選択状態が狂ってしまうのである。7のステップで「現在のタブまたはそれよりあとのタブ」が選択される場合、ここでは4のステップでタブの数が減少しているため、「次に選択するタブの番号」が「すでに削除されたタブ」のインデックスを指し示すことになり、selectedTabの値がundefinedとなりNull Pointer Errorが発生してしまう。……というのが事の真相だった。
ツリー型タブの側でselectedTabのセッタを上書きし、もし存在しないタブが選択されそうになった時は強制的に最後のタブを選択するようにしたところ、「一つもタブが選択されていない状態」は発生しなくなった。以下はそのためのコード。
// var b = gBrowser
var getter = b.__lookupGetter__('selectedTab');
var setter = b.__lookupSetter__('selectedTab');
eval('setter = '+setter.toSource().replace(
'{',
'{ if (!val) val = this.mTabContainer.lastChild;'
));
b.__defineGetter__('selectedTab', getter);
b.__defineSetter__('selectedTab', setter);
この部分ではなく、removeTabメソッドを上書きするという手もあるんだけど、ここはいろんなアドオンが上書きしたがるためコンフリクトの原因になると思ったので、「標準的な動作と変わってしまう」ことは仕方がないと割り切って、ほとんど誰も触らないであろうselectedTabの方を上書きするようにした。ちなみに、ゲッタとセッタ両方を定義し直しているのは、片方だけを再定義するともう片方が未定義になってしまうから。
表題の件に流れを戻すと、「タブを閉じる」という操作をイベントとして捕捉できるようにしたのなら、そのイベントをトリガーにして他のタブを連動して操作するという機能も当然登場してくるわけで、そういうことを考慮せずに「はいはいイベント発行すりゃいいんだろdispatchEvent っと！」と適当に設計してしまうとこういうトラブルが起こるので、APIを作る時にはもうちょっと慎重になった方がいいよね、という教訓を得たという話なのでした。
wikieditish message: Ready to edit this entry.