HashidaTKS commented 4 months ago

Which issue(s) this PR fixes:

https://github.com/ThinBridge/Chronos/issues/193

What this PR does / why we need it:

現在使用しているAlloy bootstrapはCEF127でサポート対象外となる予定。 Chrome bootstrapを使うようにする必要がある。 UIの互換性を保ちたければ、Chrome bootstrap + Alloy runtime styleにする必要がある。

このあたりは、CEF125からサポートが追加されている。

https://github.com/chromiumembedded/cef/issues/3685

CEF124以降で、Alloy bootstrapモードでの印刷のプレビューが動作しないといった問題が確認できている。 CEF127でAlloy bootstrapがサポート対象外となることから、Alloyモード特有の問題は修正されないことになっている。これもAlloyモードでしか発生しない問題なので修正されない。

https://github.com/chromiumembedded/cef/issues/3686

本PRにより、上記の印刷のプレビューが動作しない問題も修正される。

How to verify the fixed issue:

The steps to verify:

https://github.com/HashidaTKS/Chronos/actions/runs/9542955348 のChronos-stable.zip（CEF126を使用したChronos）を使用する。

10サイトほど、任意の適当なWebページを表示する
- [x] 問題なく表示されること
Googleのページを印刷する
- [x] 印刷プレビューが正しく表示されること
適当なページに遷移していき、各ページが正しく表示されること
リンクを右クリックする
- [x] コンテキストメニューが正しく表示されること（以前と変わりがないこと）
画像を右クリックする
- [x] コンテキストメニューが正しく表示されること（以前と変わりがないこと）
テキストを右クリックする
- [x] コンテキストメニューが正しく表示されること（以前と変わりがないこと）
画面上の何もない部分を右クリックする
- [x] コンテキストメニューが正しく表示されること（以前と変わりがないこと）
Googleの検索フォームを右クリックする
- [x] コンテキストメニューが正しく表示されること（以前と変わりがないこと）
任意のファイルをダウンロードする
- [x] ダウンロードの動作が以前と変わりがないこと *「インスタンスの二重起動を許可する」にチェックが入った状態にする
- ChronosDefault.confまたはChronos.confでEnableMultipleInstance=1とする
Chronosを二つ起動する
- [x] 二つ目のChronosのタブで正しく画面が表示されること

HashidaTKS commented 3 months ago

「CEF125以上」の#ifを追加しなければ...。

HashidaTKS commented 3 months ago

追加した。

kenhys commented 3 months ago

UIの互換性を保ちたければ、Chrome bootstrap + Alloy runtime styleにする必要がある。

Chrome bootstrapに追従するのはPDF previewの問題を修正するために必須というのはよいとして、 +Alloy runtime style で保たれるUIの互換性がChronosではどういった点になるのかがいまひとつよくわかりません。そのあたりを教えてもらえますか。

https://github.com/chromiumembedded/cef/issues/3685 では+Alloy runtime styleにしてもなお互換性は保たれない点があるとされています。 Alloy runtime styleにすることでデメリットが生じないかが気になっています。(+Alloy runtime style特有のバグを踏むとか)

HashidaTKS commented 3 months ago

UIの互換性を保ちたければ、Chrome bootstrap + Alloy runtime styleにする必要がある。

Chrome bootstrapに追従するのはPDF previewの問題を修正するために必須というのはよいとして、 +Alloy runtime style で保たれるUIの互換性がChronosではどういった点になるのかがいまひとつよくわかりません。そのあたりを教えてもらえますか。

chromiumembedded/cef#3685 では+Alloy runtime styleにしてもなお互換性は保たれない点があるとされています。 Alloy runtime styleにすることでデメリットが生じないかが気になっています。(+Alloy runtime style特有のバグを踏むとか)

Alloy runtime styleでなくChrome runtime styleの場合、現在のChronosと比較して以下のUIの動作差分を確認しています。（他にもあるかもしれません。名前の通り、全体的にデフォルトの動きがChromeに近くなっている感じはあります。）

ファイルダウンロード時に右上にChromeのようなダウンロード完了のポップアップが表示される
- このポップアップからファイルを開こうとするとクラッシュしたりする（HTMLファイルをダウンロードしたときに確認）
コンテキストメニューのアイテムのIDが変わるので余分なアイテムが表示されるようになる
- これはIDの変更に追随すれば直せるのだが、Alloy runtime styleならばIDが変わらないので対応不要

「互換性を保つ」という表現がよくなかったかもしれないのですが、Alloy runtime styleにするだけでこれらの動作差分を解消できるので、一旦はAlloy runtime styleにするのが良さそうに思っています。

kenhys commented 3 months ago

「互換性を保つ」という表現がよくなかったかもしれないのですが、Alloy runtime styleにするだけでこれらの動作差分を解消できるので、一旦はAlloy runtime styleにするのが良さそうに思っています。

なるほど、であればたしかにAloy runtime styleにしておくのが妥当そうです。

kenhys commented 3 months ago

TODO:

https://github.com/ThinBridge/Chronos/pull/196#pullrequestreview-2124387274
https://github.com/ThinBridge/Chronos/pull/196#issuecomment-2174991495
- ↑あたりの説明がコミットメッセージに含まれている

というのが解決したらマージしておきたい。

HashidaTKS commented 3 months ago

TODO:

* [Support chrome bootstrap alloy style #196 (review)](https://github.com/ThinBridge/Chronos/pull/196#pullrequestreview-2124387274)

* [Support chrome bootstrap alloy style #196 (comment)](https://github.com/ThinBridge/Chronos/pull/196#issuecomment-2174991495)

  * ↑あたりの説明がコミットメッセージに含まれている

というのが解決したらマージしておきたい。

コミットメッセージにこれらを含めるように修正しました。ついでにコミットを一つにまとめています。

ThinBridge / Chronos

Support chrome bootstrap alloy style #196

Which issue(s) this PR fixes:

What this PR does / why we need it:

How to verify the fixed issue:

The steps to verify: