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ヘルプをSonarCloudの解析対象に含めたことにより増えた警告に対処する #1505

Merged

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Jan 13, 2021

PR の目的

HTMLヘルプをSonarCloudの解析対象に含めたことにより増えた約3500件の警告のうち、簡単に直せるものについて暫定対処を行います。

カテゴリ

  • リファクタリング

PR の背景

#1494 でSonarCloudの解析対象範囲をC/C++以外にまで拡張した影響で、Bugsレベルの警告が4,000件近くになってしまったのを対処します。

PR のメリット

  • プロジェクトの品質とは関連性の薄いHTMLヘルプについての警告が減ります。

PR のデメリット (トレードオフとかあれば)

  • 修正は暫定対処なので、厳密に言うと正しくはない変更を含んでいます。

仕様・動作説明

アプリの仕様・機能に関する変更はありません。

変更前: HTML Work Shop が対応する古いHTMLで書かれていました。
変更後: SonarQubeが対応する最新のHTMLに移行させます。

発生している Bugs レベルの警告のうち、
framesetタグの廃止に伴う3つ以外をすべて対処します。

PR の影響範囲

HTMLヘルプの見映えに影響する可能性があります。

テスト内容

事前にSonarCloud解析で修正対象ファイル内のBugsレベル警告が3件まで減少することを確認しました。
https://sonarcloud.io/dashboard?branch=feature%2Frefactoring_of_help_html&id=berryzplus_sakura

関連 issue, PR

#1494
#1504

参考資料

・DOCTYPE宣言を付加する。
・lang="ja"を指定する。
・適切に改行する(インデントは付加しない)。
・標準は「属性名="値"」が標準だが=の前後に空白を入れてもよい。
 表記ゆれを許容すると置換しづらいので一括置換しておく。
・サイズと色が指定されたものを置換する。
・色が指定されたものを置換する。
・color="#808080"は他に合わせてgrayに置換。
・アクセシビリティの観点からtable要素には見出しを付けることになっている。
・現状はすべてのtableに見出しがない状態になっている。
・適切な名前を付けていく作業はしんどいので、一旦一律で「無題」という見出しを付ける。
・見映えの変更を防ぐためcaptionは非表示要素にしておく。
・この修正は全置換ではなく手で修正を行っている。
@sonarcloud
Copy link

sonarcloud bot commented Jan 13, 2021

SonarCloud Quality Gate failed.

Bug C 2 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 4 Security Hotspots
Code Smell A 17 Code Smells

No Coverage information No Coverage information
3.4% 3.4% Duplication

@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

Bugsレベルの検出件数が次善検証とズレてるのはある意味想定通りです。
次善検証: https://sonarcloud.io/dashboard?branch=feature%2Frefactoring_of_help_html&id=berryzplus_sakura

検知されてるSecurity Hotspotはjavascriptからのブラウザ制御に関するものです。
いつかどこかで対応したいですが放置で構わないと思います。

@berryzplus berryzplus marked this pull request as ready for review January 14, 2021 02:24
@berryzplus berryzplus added document ドキュメント SonarQube labels Jan 14, 2021
Copy link
Member

@kengoide kengoide left a comment

Choose a reason for hiding this comment

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

さらっと目で追ったぐらいの確認ですが、問題を起こす変更はなさそうだと感じました。
tableに無題captionを追加している件については強引ではないかなとも思いましたが、警告を抑えるための暫定対応としてならば別に構わない気がしています。

Copy link
Contributor

@sanomari sanomari left a comment

Choose a reason for hiding this comment

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

暫定対応ということなのでいい加減チェックです(笑)。

このヘルプファイルはサイトの一部として公開されているみたいです。
保留にされちゃってますけど、framesetをやめてググラビリティをあげる作業はやっておくのがベターだと思いました。

<frame src="./right.html" name="right">
<frameset cols="240,*" frameborder="0" border="0">
<frame src="./search.html" name="left" title="left" />
<frame src="./right.html" name="right" title="right" />
Copy link
Contributor

Choose a reason for hiding this comment

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

ここで3件のBugが検出されるはずってことですかね。
空白以外に差分のない行は編集なしとみなされたりするんでしょうか。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。
マージしてしまいます。

@berryzplus berryzplus merged commit f7fc23d into sakura-editor:master Jan 14, 2021
@berryzplus berryzplus deleted the feature/refactoring_of_help_html branch January 14, 2021 04:23
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document ドキュメント SonarQube
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants