利用規約・プライバシーポリシーの追加、独自に利用状況を収集する処理を追加 #97
Conversation
WalkthroughThe changes in this pull request include updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
js/common.js (2)
47-54: Strengthen the Google Analytics checkThe current check
if (gtag)only verifies existence but not type. Consider using a more robust check.- if (gtag) { // Google Analytics + if (typeof gtag === 'function') { // Google Analytics
57-65: Consider rate limiting survey requestsThe current implementation sends data on every timer start, which could generate excessive requests.
Consider implementing:
- Rate limiting mechanism
- Batch collection
- Sampling strategy
public/about/en.html (2)
59-70: Enhance tab navigation accessibilityThe tab navigation could be more accessible by:
- Using proper ARIA attributes
- Supporting keyboard navigation
Consider enhancing the tab structure:
- <li id="about-tab" class="is-active"> - <a onclick="activateTab('about')">Overview</a> + <li id="about-tab" class="is-active" role="tab" aria-selected="true" aria-controls="about-content"> + <a href="#about" onclick="activateTab('about'); return false;" onkeydown="if(event.key==='Enter')activateTab('about');" tabindex="0">Overview</a>
265-267: Prevent layout shifts during YouTube player initializationThe player dimensions are calculated after the page loads, which could cause Cumulative Layout Shift (CLS). Consider:
- Setting initial dimensions in CSS
- Using aspect-ratio container
Add a container with proper aspect ratio:
+ <div style="aspect-ratio: 16/9; max-width: 1024px; margin: 0 auto;"> <div id="yt-player"></div> + </div>public/about/index.html (4)
222-222: Redundant Inclusion of YouTube IFrame API ScriptThe YouTube IFrame API script is included twice:
- Line 222: Statistically included via a script tag.
- Lines 228-231: Dynamically injected into the DOM.
This redundancy may lead to unnecessary overhead or potential conflicts.
Consider removing one of the inclusions to optimize page load performance. If the dynamic loading is not required, you can remove the dynamic script injection:
Alternatively, if dynamic loading is preferred, remove the static script inclusion:
Also applies to: 228-231
87-87: Enhance Alt Text for Images to Improve AccessibilityThe
altattributes for the images are currently generic (e.g., "SyncTimer1"), which may not provide meaningful information for users utilizing screen readers.Consider providing more descriptive
alttext to improve accessibility:+ <img src="images/SyncTimer1.webp" alt="SyncTimer displaying negative start time" /> + <img src="images/SyncTimer2.webp" alt="SyncTimer with chroma key compositing and fixed-width display" /> + <img src="images/SyncTimer3.webp" alt="SyncTimer preserving settings via URL" />Also applies to: 97-97, 109-109
11-16: Optimize Font Awesome ImportsMultiple Font Awesome CSS files are being imported separately:
- Lines 11-13:
fontawesome.min.css- Lines 14-16:
brands.min.cssTo reduce HTTP requests and improve page load times, consider consolidating these imports. You can import the complete Font Awesome package if necessary:
- <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.3.0/css/fontawesome.min.css" - integrity="sha512-cHxvm20nkjOUySu7jdwiUxgGy11vuVPE9YeK89geLMLMMEOcKFyS2i+8wo0FOwyQO/bL8Bvq1KMsqK4bbOsPnA==" - crossorigin="anonymous" referrerpolicy="no-referrer" /> - <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.3.0/css/brands.min.css" - integrity="sha512-L+sMmtHht2t5phORf0xXFdTC0rSlML1XcraLTrABli/0MMMylsJi3XA23ReVQkZ7jLkOEIMicWGItyK4CAt2Xw==" - crossorigin="anonymous" referrerpolicy="no-referrer" /> + <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.3.0/css/all.min.css" + integrity="sha512-... (updated integrity hash) ..." + crossorigin="anonymous" referrerpolicy="no-referrer" />Ensure to update the integrity hash accordingly.
74-123: Consider Lazy Loading YouTube Player for PerformanceThe YouTube player is embedded directly, which may impact the initial page load time.
Consider implementing lazy loading of the YouTube player to improve performance. You can load the player only when the user scrolls to that section or interacts with it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.gitignore(1 hunks)js/common.js(1 hunks)public/about/en.html(1 hunks)public/about/index.html(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🔇 Additional comments (3)
js/common.js (1)
57-65:
Review data collection for potential PII exposure
The code collects host information and user preferences which could potentially be used for user fingerprinting.
public/about/en.html (2)
28-41: Verify Google Analytics initialization before usage
The activateTab function uses gtag directly without checking if Google Analytics has been properly initialized. This could lead to errors if the GA script hasn't loaded yet.
Consider wrapping the gtag call in a check:
function activateTab(to) {
const tabs = document.querySelectorAll(".tabs li");
const tabContents = document.getElementsByClassName("tab-content");
for (const tab of tabs) {
tab.className = "";
if (tab.id == to + "-tab") tab.className = "is-active";
}
for (const content of tabContents) {
content.style.display = "none";
if (content.id == to + "-content") content.style.display = "block";
}
- gtag("event", "activate_tab", { event_label: to });
+ if (typeof gtag === 'function') {
+ gtag("event", "activate_tab", { event_label: to });
+ }
}202-210: Verify data collection implementation details
The Privacy Policy mentions collecting "configuration information logs" via proprietary methods. This aligns with the PR objective but requires careful implementation to ensure:
- Only non-PII data is collected
- Data collection is transparent to users
- Users have opt-out options
| // Send timer settings to my analytics on timer start. | ||
| if (`${import.meta.env.VITE_SURVEY_URL}` !== "" && action === "sync_timer_start") { |
There was a problem hiding this comment.
Add user consent mechanism for data collection
The code collects user settings and host information without explicit user consent, which may violate privacy regulations (GDPR, CCPA).
Consider implementing:
- A consent management system
- Clear user notification about data collection
- Option to opt-out of analytics
| fetch(`${import.meta.env.VITE_SURVEY_URL}?${params.toString()}`).then((res) => { | ||
| if (res.ok) { | ||
| console.debug("Survey request sent."); | ||
| } else { | ||
| console.log("Survey request failed."); | ||
| } | ||
| }).catch((err) => { | ||
| console.log("Survey request failed.", err); | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and URL validation
Several issues need attention:
- Console.log statements in production code
- No validation of VITE_SURVEY_URL
- Basic error handling
+ const surveyUrl = import.meta.env.VITE_SURVEY_URL;
+ if (!surveyUrl || !surveyUrl.startsWith('https://')) {
+ console.error('Invalid survey URL configuration');
+ return;
+ }
- fetch(`${import.meta.env.VITE_SURVEY_URL}?${params.toString()}`).then((res) => {
+ fetch(`${surveyUrl}?${params.toString()}`).then((res) => {
if (res.ok) {
- console.debug("Survey request sent.");
+ // Success case - consider adding metrics instead of console logs
} else {
- console.log("Survey request failed.");
+ // Use proper error tracking service
+ throw new Error(`Survey request failed: ${res.status}`);
}
}).catch((err) => {
- console.log("Survey request failed.", err);
+ // Use proper error tracking service
+ reportError('Survey request failed', err);
})| <script src="https://www.youtube.com/iframe_api"></script> | ||
| <script> | ||
| const elem = document.getElementById("yt-player"); | ||
| const ytWidth = Math.min(elem.parentNode.clientWidth, 1024); | ||
| const ytHeight = (ytWidth / 16) * 9; | ||
|
|
||
| const tag = document.createElement("script"); | ||
| tag.src = "https://www.youtube.com/iframe_api"; | ||
| const firstScriptTag = document.getElementsByTagName("script")[0]; | ||
| firstScriptTag.parentNode.insertBefore(tag, firstScriptTag); | ||
| const tag = document.createElement("script"); | ||
| tag.src = "https://www.youtube.com/iframe_api"; | ||
| const firstScriptTag = document.getElementsByTagName("script")[0]; |
There was a problem hiding this comment.
Remove duplicate YouTube iframe API script load
The YouTube iframe API script is loaded twice:
- Line 263:
<script src="https://www.youtube.com/iframe_api"></script> - Lines 269-271: Manual script tag creation
Remove one of the script loads to prevent potential issues:
- <script src="https://www.youtube.com/iframe_api"></script>
<script>
const elem = document.getElementById("yt-player");
const ytWidth = Math.min(elem.parentNode.clientWidth, 1024);
const ytHeight = (ytWidth / 16) * 9;
const tag = document.createElement("script");
tag.src = "https://www.youtube.com/iframe_api";
const firstScriptTag = document.getElementsByTagName("script")[0];
firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <script src="https://www.youtube.com/iframe_api"></script> | |
| <script> | |
| const elem = document.getElementById("yt-player"); | |
| const ytWidth = Math.min(elem.parentNode.clientWidth, 1024); | |
| const ytHeight = (ytWidth / 16) * 9; | |
| const tag = document.createElement("script"); | |
| tag.src = "https://www.youtube.com/iframe_api"; | |
| const firstScriptTag = document.getElementsByTagName("script")[0]; | |
| firstScriptTag.parentNode.insertBefore(tag, firstScriptTag); | |
| const tag = document.createElement("script"); | |
| tag.src = "https://www.youtube.com/iframe_api"; | |
| const firstScriptTag = document.getElementsByTagName("script")[0]; | |
| <script> | |
| const elem = document.getElementById("yt-player"); | |
| const ytWidth = Math.min(elem.parentNode.clientWidth, 1024); | |
| const ytHeight = (ytWidth / 16) * 9; | |
| const tag = document.createElement("script"); | |
| tag.src = "https://www.youtube.com/iframe_api"; | |
| const firstScriptTag = document.getElementsByTagName("script")[0]; |
| <h3 class="title">第6条（利用規約の変更）</h3> | ||
| <p>開発者は、必要と判断した場合、ユーザーに通知することなく本規約を変更することができるものとします。なお、本規約の変更後、本サービスの利用を開始した場合には、ユーザーは変更後の規約に同意したものとみなされます。</p> | ||
| <h3 class="title">第7条（個人情報の取扱い）</h3> | ||
| <p>開発者は、本サービスの利用によって取得する個人情報については、<a onclick="activateTab('policy')">プライバシーポリシー</a>に従い適切に取り扱うものとします。</p> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid Inline JavaScript in Anchor Tags
The anchor tag at line 153 uses the onclick attribute to trigger JavaScript:
<a onclick="activateTab('policy')">プライバシーポリシー</a>For better separation of concerns and to enhance security, consider adding an event listener in your script instead of using inline JavaScript. This approach can help prevent potential XSS vulnerabilities and improve code maintainability.
Example:
- Add an
idorclassto the anchor tag:
+ <a id="privacy-policy-link">プライバシーポリシー</a>- In your script, add an event listener:
+ <script>
+ document.getElementById("privacy-policy-link").addEventListener("click", function(event) {
+ event.preventDefault();
+ activateTab('policy');
+ });
+ </script>Repeat this pattern for other instances where onclick is used within HTML.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
.gitignoreto exclude sensitive.envfiles.