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

HTML内のMFMっぽい部分を<plain>でエスケープするように #15241

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

anatawa12
Copy link
Member

What

HTML内のMFMっぽい部分をでエスケープするようにします。

Why

Fix #15217

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/backend:test labels Jan 9, 2025
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 89.65517% with 9 lines in your changes missing coverage. Please review.

Project coverage is 40.34%. Comparing base (13439e0) to head (a32045f).

Files with missing lines Patch % Lines
packages/backend/src/@types/twemoji.d.ts 0.00% 8 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #15241      +/-   ##
===========================================
+ Coverage    40.31%   40.34%   +0.02%     
===========================================
  Files         1564     1566       +2     
  Lines       198083   198165      +82     
  Branches      3837     3858      +21     
===========================================
+ Hits         79863    79946      +83     
+ Misses      117617   117615       -2     
- Partials       603      604       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jan 9, 2025

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@@ -194,8 +197,8 @@ export class MfmService {
case 'blockquote': {
const t = getText(node);
if (t) {
text += '\n> ';
text += t.split('\n').join('\n> ');
// TODO: HTML in blockquote are not proceed correctly
Copy link
Member Author

Choose a reason for hiding this comment

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

ここらの TODO は別 PR で #15218 の対応と一緒にやりたいなと思ってます。(このTODOの修正にNode -> MFMの関数が欲しくて、 #15218 の対応でもほしいと思ったため)

@syuilo
Copy link
Member

syuilo commented Jan 9, 2025

mastodon等 MFM を使用しないサーバーからのノートに含まれる **> などが MFM として解釈されていた問題を修正

これは意図してるのよね

@anatawa12
Copy link
Member Author

意図しているので修正しないというのであればそのような旨で #15217 を close as not planned していただきたいです。
(openなので何かしら修正するつもりと解釈してましたすみません)

@syuilo
Copy link
Member

syuilo commented Jan 9, 2025

issueがあるということなので何かしらの修正は要ると思うけど、何をどう修正するかが決定していなかった気がする(あまり追えてない)

@anatawa12
Copy link
Member Author

issueの目的はHTMLの>**をMFMとして処理してほしくないということなので、

mastodon等 MFM を使用しないサーバーからのノートに含まれる **> などが MFM として解釈されていた問題を修正

これは意図してるのよね

とは真っ向から対立している issue なのでこれを通すのであれば close as not planned だと思います。
(部分的に修正の可能性はありますが)

issueがあるということなので何かしらの修正は要ると思う

に関してはユーザが問題視したけどプロジェクトの方針として問題としないのであれば修正しないが正しいことがあり得ると思っています。そして今回はこれに該当する可能性があると思います。

@sakuhanight
Copy link
Contributor

方向性としてはこのコメントの通りと思います。
この 「MFMとして解釈されてしまう」 を意図通りとするならば、anatawa12さんのとおりclose as not plannedだと思います。

catsmiry added a commit to catsmiry/misskey that referenced this pull request Jan 9, 2025
Signed-off-by: anatawa12 <anatawa12@icloud.com>

chore: move escapeMFM to static function of MFMService (for testing)

fix: :emoji: and unicode emojis are not proceeded correctly

test: add tests for MFM Escaping

refactor: move `splitSegments` to another file

docs: missing license header

fix: import path doesn't have '.js' extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend:test packages/backend Server side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

ActivityPub: _misskey_contentがない投稿にはHTMLとしての処理だけを行うべき
3 participants