コンボボックスのサブクラス化に関連するクラッシュバグの修正

[kengoide]

PR の目的

「ファイルを開く」および「名前を付けて保存」ダイアログにて、「最近のファイル」・「最近のフォルダ」コンボボックスにフォーカスがある状態で Ctrl+Backspace を押すとクラッシュします。
#1470 の機能追加による劣化バグで、本PRはその修正が目的です。

カテゴリ

• 不具合修正

PR の背景

問題の原因ですが、コンボボックスをサブクラス化する処理が CBS_DROPDOWNLIST （編集できないコンボボックス）を考慮していなかったことです。
SetComboBoxDeleter 関連のコードは、API関数 GetComboBoxInfo を通じて コンボボックス内部の Edit のハンドルを入手できることを前提としています。編集可能なコンボボックスでは期待通りにEditを取得できますが、CBS_DROPDOWNLIST スタイルのコンボボックスに対して同じことをすると結果は ComboBox のハンドルになってしまいます。このケースを考慮できていませんでした。

sakura/sakura_core/dlg/CDialog.cpp

Lines 813 to 816 in 526f512

	COMBOBOXINFO info = { sizeof(COMBOBOXINFO) };
	if (!::GetComboBoxInfo(hwndCtl, &info))
	return;
	::SetWindowSubclass(info.hwndItem, SubEditProc, 0, (DWORD_PTR)pRecent);

hwndCtl が CBS_DROPDOWNLIST スタイルのコンボボックスだった場合、info.hwndItem に ComboBox が入った状態で返ってくるため、Edit をサブクラス化するつもりで ComboBox をサブクラス化することになります。

sakura/sakura_core/dlg/CDialog.cpp

Lines 767 to 770 in 526f512

	LRESULT CALLBACK SubEditProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam,
	UINT_PTR uIdSubclass, DWORD_PTR dwRefData)
	{
	HWND hwndCombo = GetParent(hwnd);

ここで Edit の親（＝ComboBox）を取得しようとして ComboBox の親（＝ダイアログ）を取得してます。

sakura/sakura_core/dlg/CDialog.cpp

Lines 781 to 785 in 526f512

	case WM_CHAR:
	// ASCII 削除文字。Ctrl + Backspace が入力された。
	if (wParam == 0x7f) {
	DWORD selStart, selEnd;
	Combo_GetEditSel(hwndCombo, selStart, selEnd);

hwndCombo はダイアログです。CB_GETEDITSELには反応してくれません。selStartとselEndは未初期化のままです。

sakura/sakura_core/dlg/CDialog.cpp

Lines 793 to 798 in 526f512

	// 単語削除する
	const int length = ::GetWindowTextLength(hwndCombo);
	auto text = std::make_unique<wchar_t[]>(length + 1);
	::GetWindowText(hwndCombo, text.get(), length + 1);
	const int pos = DeletePreviousWord(text.get(), length, selStart);

length は 0、textはL'\0'が入った長さ1の配列となります。selStartが偶然0になっていなければDeletePreviousWordの中で Access Violation が発生します😢

仕様・動作説明

問題に対する対処として、CBS_DROPDOWNLIST スタイルのコンボボックスに SetComboBoxDeleter を適用しないことにします。該当するのは「ファイルを開く」および「名前を付けて保存」ダイアログ下部のそれです。
副作用として履歴削除が機能しなくなる…と思いきや、実は別バグ（後述）により CBS_DROPDOWNLIST スタイルのコンボボックスでは元から機能していなかったため、機能面では退行しないものと思われます。

PR の影響範囲

以下のダイアログに存在したクラッシュバグを修正します。

• ファイルを開く・名前を付けて保存（Vistaスタイルを除く）

以下の画面に存在するコンボボックスに関係するコードを変更します。

• 検索・置換
• Grep・Grep置換
• 外部コマンド実行
• ツールバーの検索ボックス

テスト内容

• 「共通設定」→「編集」→「Vistaスタイルのファイルダイアログ」にチェックが入っていない状態で、開く・保存ダイアログに対して以下のテストが必要です

  • 「最近のファイル」・「最近のフォルダ」をクリックするなどしてフォーカスを移動させる
  • Ctrl + Backspace を押下する
  • プログラムがクラッシュしないことを確認する

• 他の画面の該当コンボボックス（上記）に対して、履歴削除・単語削除がそれぞれ引き続き機能することの確認が必要です。

関連 issue, PR

#1463, #1470

参考資料

履歴削除の別バグ

sakura/sakura_core/dlg/CDialog.cpp

Lines 725 to 738 in 526f512

	// コンボボックスのキャレット位置を取得
	DWORD dwSelStart = 0;
	DWORD dwSelEnd = 0;
	Combo_GetEditSel( hwndCombo, dwSelStart, dwSelEnd );
	// アイテムテキストとエディットテキストが異なる、またはエディットが全選択でなかった場合
	if ( cItemText != cEditText
	|| 0 < dwSelStart
	|| dwSelEnd < (DWORD)cEditText.GetStringLength()
	)
	{
	// 履歴削除をスキップする
	return;
	}

CBS_DROPDOWNLIST スタイルのコンボボックスは CB_GETEDITSEL が送られてきても何もしないようで、dwSelStartとdwSelEndは常に0のまま変わりません。cEditTextは最短でも長さ1（null文字の分）であるため、dwSelEnd < (DWORD)cEditText.GetStringLength()の条件式により、履歴削除が実行されることはありません。
本 PR では元から動いていなかったことを理由として機能を削除しますが、対処方針として適切かどうかというと微妙だと思っています。後から追加PRとして修正することも考えていますので、問題があればご指摘ください。

[AppVeyorBot]

✅ Build sakura 1.0.3287 completed (commit 6073a73ce1 by @k-kagari)

[berryzplus]

指摘2件ありますが、クラッシュバグの回復が優先だと思うのでapproveにしておきます。

[kengoide]

Approvalをいただきましたが、PRの出し直しを避けたいため先に指摘に対応させてください。

[AppVeyorBot]

✅ Build sakura 1.0.3288 completed (commit 25aff663e3 by @k-kagari)

[kengoide]

承認ありがとうございます。