feat: Improve Push Notification #7667

Merged

tamaina merged 522 commits from swn into develop

tamaina commented (Migrated from github.com)

Summary

Resolve #6359, Resolve #6628, Resolve #6395

#7129 でgitの誤操作によりブランチが蒸発したため新規にPRを出します。
かなり長期間にわたっていますので、#7129も参照してください。

What

Service Workerのプッシュ通知を拡張し、actionsを用いて通知に反応できるようにします。
また、プッシュ通知まわりの挙動の整理もします。

Why

Service Workerのプッシュ通知でいろいろできるので

依存PR

Waiting to merge

#7605,

Merged

#7108, #7196, #7609, #7611, #7750, #7606, #7928, #7929

Changes

PushのデータにuserIdを追加して、通知を受け取るアカウントのIDを識別できるように
actions用にpostMessageでフォームを開いてWindowClient.focus()でタブを開く機能
actionsの実装・翻訳
ユーザー/グループとの会話がすべて既読になったらSW Pushに対してreadAllMessagingMessagesOfARoomを発行するように
既存のタブで開く場合は、通常のページなのか、ウィンドウで開くかサイドビューで開くか、を判断するように
アプリが生成する通知をSWで受信できるようにする
package分離、ビルドはesbuildで

バグ（仕様です）

pushが来て何も通知が出ないと「このサイトはバックグラウンドで更新されました」が出る
- setTimeoutで消すようにはした
WindowsのFirefoxだと、一定時間経過すると自動でclose()が発火するため、だいたいの通知が既読になってしまう
~~プッシュ通知のペイロードが長すぎるとブラウザのプッシュ通知エンドポイントへ送信する際にエラーとなる~~
- ペイロードを短くするようにした

将来的な拡張（今回のPRでは行わない）

Web UIから設定できるようにする (#6360)
- Firefoxの自動close対策もここで可能になるかなと思う
- オンオフや音の設定
#8012 バッジを指定するように (#7206)

## Summary Resolve #6359, Resolve #6628, Resolve #6395 #7129 でgitの誤操作によりブランチが蒸発したため新規にPRを出します。かなり長期間にわたっていますので、#7129も参照してください。 # What Service Workerのプッシュ通知を拡張し、actionsを用いて通知に反応できるようにします。また、プッシュ通知まわりの挙動の整理もします。 # Why Service Workerのプッシュ通知でいろいろできるので ## 依存PR ### Waiting to merge #7605, ### Merged #7108, #7196, #7609, #7611, #7750, #7606, #7928, #7929 ## Changes - [x] PushのデータにuserIdを追加して、通知を受け取るアカウントのIDを識別できるように - [x] actions用にpostMessageでフォームを開いてWindowClient.focus()でタブを開く機能 - [x] actionsの実装・翻訳 - [x] ユーザー/グループとの会話がすべて既読になったらSW Pushに対してreadAllMessagingMessagesOfARoomを発行するように - [x] 既存のタブで開く場合は、通常のページなのか、ウィンドウで開くかサイドビューで開くか、を判断するように - [x] アプリが生成する通知をSWで受信できるようにする - [x] package分離、ビルドはesbuildで ## バグ（仕様です） 1. pushが来て何も通知が出ないと「このサイトはバックグラウンドで更新されました」が出る * setTimeoutで消すようにはした 2. WindowsのFirefoxだと、一定時間経過すると自動でclose()が発火するため、だいたいの通知が既読になってしまう 3. ~~プッシュ通知のペイロードが長すぎるとブラウザのプッシュ通知エンドポイントへ送信する際にエラーとなる~~ * ペイロードを短くするようにした ## 将来的な拡張（今回のPRでは行わない） - [ ] Web UIから設定できるようにする (#6360) * Firefoxの自動close対策もここで可能になるかなと思う * オンオフや音の設定 - [x] #8012 バッジを指定するように (#7206)

tamaina commented (Migrated from github.com)

IndexedDBが何かの拍子に壊れるっぽくて頭を抱えている
（idb-keyvalを通して弄っているので不正な操作はしていないはず…

IndexedDBが何かの拍子に壊れるっぽくて頭を抱えている（idb-keyvalを通して弄っているので不正な操作はしていないはず…

tamaina commented (Migrated from github.com)

mark-all-as-read叩いてもインジケータが消えない

tamaina commented (Migrated from github.com)

mark-all-as-read叩いてもインジケータが消えない

直した

IndexedDBが何かの拍子に壊れる

再現性がないのでとりあえずパス

> mark-all-as-read叩いてもインジケータが消えない直した > IndexedDBが何かの拍子に壊れる再現性がないのでとりあえずパス

tamaina commented (Migrated from github.com)

直した

直ってないんじゃないかな

> 直した直ってないんじゃないかな

tamaina commented (Migrated from github.com)

p1.a9z.devだと

readNotificationでは、すべてが既読になったはずなのにreadAllNotificationではなくreadNotificationが発行されるらしい
mark-all-as-readを叩いてもreadAllNotificationがstreamされない

んだけど、他のPR由来でDBがぶっ壊れてる可能性もある

p1.a9z.devだと - readNotificationでは、すべてが既読になったはずなのにreadAllNotificationではなくreadNotificationが発行されるらしい - mark-all-as-readを叩いてもreadAllNotificationがstreamされないんだけど、他のPR由来でDBがぶっ壊れてる可能性もある

tamaina commented (Migrated from github.com)

他のPR由来だった

tamaina commented (Migrated from github.com)

IndexedDBが何かの拍子に壊れる

いまのところ壊れていないので、修正前のコードで作成したDBを直していないケースだったんじゃないかと思う

> IndexedDBが何かの拍子に壊れるいまのところ壊れていないので、修正前のコードで作成したDBを直していないケースだったんじゃないかと思う

tamaina commented (Migrated from github.com)

Misskey.jsのapiクライアントを使ったらChromiumだとエラーがでる…

Misskey.jsのapiクライアントを使ったらChromiumだとエラーがでる… ![image](https://user-images.githubusercontent.com/7973572/134052278-a2eabc6a-dadc-4883-afa1-8337ddedd8b7.png) ![image](https://user-images.githubusercontent.com/7973572/134052373-e4366369-2a9c-490d-953c-65911ce07af6.png)

syuilo commented (Migrated from github.com)

謎

tamaina commented (Migrated from github.com)

あ、いやなんとなく原因が分かった

https://github.com/tamaina/misskey/blob/swn/src/client/sw/operations.ts#L13

このfetchの渡し方は禁忌だった気がする

あ、いやなんとなく原因が分かった https://github.com/tamaina/misskey/blob/swn/src/client/sw/operations.ts#L13 このfetchの渡し方は禁忌だった気がする

syuilo commented (Migrated from github.com)

misskey.js側でinServiceWorkerみたいなオプション実装すれば回避できるかしら

misskey.js側で`inServiceWorker`みたいなオプション実装すれば回避できるかしら

tamaina commented (Migrated from github.com)

SWと通常のfetchって変わらなかった気がするので特別指定しなくてもいい気もする

syuilo commented (Migrated from github.com)

SerivceWorkerってwindowあったっけ？

tamaina commented (Migrated from github.com)

windowはない

syuilo commented (Migrated from github.com)

window省略表記すればよしなになるのか

tamaina commented (Migrated from github.com)

現状window呼び出してないから大丈夫そう

https://github.com/misskey-dev/misskey.js/blob/develop/src/api.ts

でもfetch指定しなくてもIllegal invocation出たわ…

現状window呼び出してないから大丈夫そう https://github.com/misskey-dev/misskey.js/blob/develop/src/api.ts でもfetch指定しなくてもIllegal invocation出たわ…

tamaina commented (Migrated from github.com)

メモ: プッシュ通知のペイロードが長すぎるとブラウザのプッシュ通知エンドポイントがエラーを返すっぽい

tamaina commented (Migrated from github.com)

禁忌

fetchなどのビルトイン関数は変数に関数としてそのまま代入するとエラーになる（とくにChromiumでは）
なので、

{ fetch: (...args) => fetch(...args) }

のように即席関数でラップしてやると良くなった

> 禁忌 fetchなどのビルトイン関数は変数に関数としてそのまま代入するとエラーになる（とくにChromiumでは）なので、 { fetch: (...args) => fetch(...args) } のように即席関数でラップしてやると良くなった

👍 1

tamaina commented (Migrated from github.com)

(misskey.jsでもこれやっといたほうがいいかも？(私はもう寝るけど

👍 1

tamaina commented (Migrated from github.com)

起きたのでmisskey.jsにプルリク作った

https://github.com/misskey-dev/misskey.js/pull/29

起きたのでmisskey.jsにプルリク作った https://github.com/misskey-dev/misskey.js/pull/29

🚀 1

tamaina commented (Migrated from github.com)

webpackを使っているせいで警告されるようになった

webpackを使っているせいで警告されるようになった ![image](https://user-images.githubusercontent.com/7973572/138593818-155baf4e-0e1d-4a09-8ada-a0eb2668d16d.png)

tamaina commented (Migrated from github.com)

Androidで通知が表示されないなぁと不思議に思っていたら単にサイレントモードが有効になっていただけだった

syuilo (Migrated from github.com) reviewed

syuilo (Migrated from github.com) left a comment

packages/client/src/scripts/get-note-summary.ts
の変更は全て不要かも

~~packages/client/src/scripts/get-note-summary.ts の変更は全て不要かも~~

syuilo commented (Migrated from github.com)

clinetのscriptsって、ローカルストレージだったりVueだったりに依存している処理があるから、ServiceWorkerのコードはまるごとswパッケージにでもして分離した方が分かりやすくなりそうとちょっと思ったけどどうだろう

👍 1

tamaina commented (Migrated from github.com)

swパッケージにでもして分離した方が分かりやすくなりそう

これやるか。Service Workerの追加はクライアントをもう一つ増やすのと変わりないので。

> swパッケージにでもして分離した方が分かりやすくなりそうこれやるか。Service Workerの追加はクライアントをもう一つ増やすのと変わりないので。

tamaina (Migrated from github.com) reviewed

syuilo (Migrated from github.com) reviewed

tamaina (Migrated from github.com) reviewed

syuilo (Migrated from github.com) reviewed

tamaina (Migrated from github.com) reviewed

tamaina commented (Migrated from github.com)

Failed to load the script unexpectedlyでスクリプトを起動できないんだけど…

![image](https://user-images.githubusercontent.com/7973572/143895834-c805c2fe-80d6-45d0-9fa5-a309675b3112.png) `Failed to load the script unexpectedly`でスクリプトを起動できないんだけど…

tamaina commented (Migrated from github.com)

Chrome/EdgeでOS関係なく発生する、Firefoxだとちゃんと動いている

tamaina commented (Migrated from github.com)

エラーコードでググったらソースコード出てきたわ

19cda129dc/third_party/blink/renderer/core/loader/modulescript/installed_service_worker_module_script_fetcher.cc (44)

エラーコードでググったらソースコード出てきたわ https://chromium.googlesource.com/chromium/src/+/19cda129dc0b7cbb8a5c982b9f85455f1363dcfb/third_party/blink/renderer/core/loader/modulescript/installed_service_worker_module_script_fetcher.cc#44

tamaina commented (Migrated from github.com)

ルートにself.addEventListenerが来ていないのが原因なような気はするけど、Webpackでそういうことができるのかどうかはわからない…