Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

カスタム絵文字をproxyに通すように #7526

Merged
merged 1 commit into from
May 27, 2021
Merged

Conversation

rinsuki
Copy link
Contributor

@rinsuki rinsuki commented May 21, 2021

Summary

Resolve #6451

試してないけどたぶん動く

@rinsuki rinsuki force-pushed the feat/proxy-emoji branch from 919b7fb to 8935ab4 Compare May 21, 2021 10:15
@rinsuki rinsuki requested a review from syuilo May 27, 2021 13:39
@syuilo syuilo merged commit 466c083 into develop May 27, 2021
@syuilo syuilo deleted the feat/proxy-emoji branch May 27, 2021 13:40
@syuilo
Copy link
Member

syuilo commented May 27, 2021

LGTM

@@ -59,9 +61,12 @@ export async function populateEmoji(emojiName: string, noteUserHost: string | nu

if (emoji == null) return null;

const isLocal = emojiName.endsWith('@.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ユーザー情報についてくる絵文字情報はローカルでも@.が付かないから判定条件変えないとダメだな

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

うーん判定が難しい

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nameの形式でも判定できないし(foo形式の文字列はローカルでもリモートでもありうる)、urlでも判定できない(外部のオブジェクトストレージとか使ってる可能性ある)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

まあ別にローカルなURLをさらにproxyに通しても問題は無いんだけど、無駄ではある

Copy link
Contributor

@mei23 mei23 May 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すぐ上でDBクエリ用にhostをresolveしているので、普通にhostがnullかどうかで判定すればOK。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ほんとだ

const isLocal = emoji.host == null;

でいいかな

@syuilo
Copy link
Member

syuilo commented May 28, 2021

@rinsuki @mei23 @acid-chicken ↑良いアイデア募集中

@mei23
Copy link
Contributor

mei23 commented May 28, 2021

/files/:name@:host/:timestamp.pngのルートでProxyするといい感じ。
(未保存リモートファイルのプロキシのカスタム絵文字版)

@mei23
Copy link
Contributor

mei23 commented May 28, 2021

あれ、その部分じゃない?
なんか大丈夫だった気がするから後で見てみる

syuilo added a commit that referenced this pull request May 30, 2021
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this pull request Sep 24, 2021
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this pull request Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

外部のカスタム絵文字が直リンになる
3 participants