Conversation
- 引入共享的主题初始化脚本，用于根据用户偏好或本地存储设置主题
ChuwuYo
added
bug
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors multiple UI modules to replace string-based innerHTML assembly with DOM creation using DocumentFragment; adds a theme-init module that applies theme via documentElement, changes i18n language detection to use Changes
Sequence Diagram(s)(omitted — changes are UI rendering refactors and small init scripts; no new multi-component sequential feature introduced) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ChuwuYo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求主要致力于提升扩展的用户界面性能和视觉一致性。通过将HTML内容的生成方式从字符串拼接转向更高效、安全的DOM API操作，以及引入一个专门的主题初始化脚本来消除主题闪烁，这些改变共同为用户带来了更流畅、更稳定的交互体验。同时，版本号的更新也反映了这些重要的内部改进。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 10 files
Confidence score: 4/5
- Invalid
<hr>placement inside<ul>indebug-ui.jscould yield inconsistent rendering across browsers despite being mostly cosmetic. shared/theme-init.jsaccepts any stored theme string, so a bad value from localStorage can break the theme until storage is cleared.- Pay close attention to
debug-ui.js,shared/theme-init.js– ensure markup validity and theme value validation before landing.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="debug-ui.js">
<violation number="1" location="debug-ui.js:157">
P2: `<hr>` 不是 `<ul>` 的有效子元素。如果需要在规则之间添加分隔线，应该使用带有分隔样式的 `<li>` 元素，或者通过 CSS 在 `<li>` 上添加边框样式。</violation>
</file>
<file name="shared/theme-init.js">
<violation number="1" location="shared/theme-init.js:5">
P2: 从 localStorage 读取的主题值缺少有效性验证。如果存储了无效值（如 'invalid'），会被直接使用，导致主题样式可能失效。建议验证 savedTheme 是否为 'dark' 或 'light' 之一。</violation>
</file>
Architecture diagram
sequenceDiagram
participant User
participant DOM as Browser Page (DOM)
participant Theme as shared/theme-init.js
participant Script as UI Scripts (Popup/Debug)
participant API as Chrome Runtime
Note over User,Theme: NEW: Immediate Theme Initialization (Fixes Flicker)
User->>DOM: Open Extension Page
DOM->>Theme: Load & Execute (Synchronous)
Theme->>DOM: NEW: Check localStorage & matchMedia
Theme->>DOM: NEW: setAttribute('data-bs-theme') on <html>
Note over DOM,Script: Load Main Application
DOM->>Script: DOMContentLoaded
Script->>API: Fetch Data (Rules/Headers/Version)
API-->>Script: Return JSON Data
Note over Script,DOM: CHANGED: Secure DOM Generation (was innerHTML)
Script->>DOM: Clear container (innerHTML = '')
Script->>Script: NEW: createDocumentFragment()
loop Build UI Elements
Script->>Script: NEW: createElement()
Script->>Script: NEW: set textContent / className
Note right of Script: Avoids string concat<br/>Prevents XSS
Script->>Script: NEW: fragment.appendChild(element)
end
Script->>DOM: NEW: container.appendChild(fragment)
Note left of DOM: Single Reflow / Paint
DOM-->>User: Rendered View
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- 调整规则分隔线样式，使用li元素替代hr - 简化DOM操作，使用append方法替代多次appendChild - 修复主题初始化逻辑，严格检查存储的主题值
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="toggle.css">
<violation number="1" location="toggle.css:188">
P3: 文件末尾缺少换行符。POSIX 规范要求文本文件以换行符结尾，缺少换行符可能导致某些工具出现问题，且 Git 也会对此发出警告。</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
debug-ui.js (1)
674-675: Inconsistent event listener registration.Line 675 uses
document.getElementById(...).addEventListener(...)directly instead ofResourceManager.addEventListener(). This is inconsistent with the rest of the file (e.g., lines 83, 290, 332, 456, 500, 614, 694, 832, 858) and may cause the listener to not be cleaned up duringResourceManager.cleanup().🐛 Suggested fix
// 重置自定义语言设置 - document.getElementById('resetCustomLangBtn').addEventListener('click', async () => { + ResourceManager.addEventListener(document.getElementById('resetCustomLangBtn'), 'click', async () => {
🧹 Nitpick comments (4)
debug-ui.js (4)
40-55: Consider usingreplaceChildren()for DOM-consistent clearing.The
setSafeContentfunction usesinnerHTML = ''to clear content. While safe, this is inconsistent with the PR's goal of using DOM APIs exclusively. ThereplaceChildren()method achieves the same result with pure DOM manipulation.♻️ Suggested refactor
const setSafeContent = (element, content, className = '') => { // 清空现有内容 - element.innerHTML = ''; + element.replaceChildren(); if (Array.isArray(content)) {
83-85: ReplaceinnerHTMLwithtextContentfor loading message.Line 85 uses
innerHTMLto set a loading message from i18n. Since this is plain text without HTML markup, usingtextContentwould be more consistent with the PR's DOM-based approach and provides an additional layer of safety.♻️ Suggested fix
ResourceManager.addEventListener(document.getElementById('showRulesBtn'), 'click', () => { const resultElement = document.getElementById('rulesResult'); - resultElement.innerHTML = debugI18n.t('getting_rule_info'); + resultElement.textContent = debugI18n.t('getting_rule_info');
693-829: Consider restructuring the async flow for clarity.The diagnostics function has a complex flow where a
DocumentFragmentis created synchronously, populated with manifest info, and then an async IIFE adds storage data before finally appending the fragment to the DOM. While this works correctly, the structure could be clearer.The current pattern means the fragment is only appended inside the async callback (line 809), so any failure in the async portion could leave the synchronous data un-rendered, even though it was collected successfully.
♻️ Suggested approach
Consider either:
- Making the entire handler async and awaiting the storage call directly, or
- Appending the synchronous content immediately, then appending async content separately
ResourceManager.addEventListener(document.getElementById('showDiagnosticsBtn'), 'click', () => { + // Alternative: make the entire handler async + ResourceManager.addEventListener(document.getElementById('showDiagnosticsBtn'), 'click', async () => { const resultElement = document.getElementById('diagnosticsResult'); - resultElement.innerHTML = debugI18n.t('collecting_diagnostics'); + resultElement.textContent = debugI18n.t('collecting_diagnostics'); // ... then use await instead of nested async IIFE
458-458: Replace remaininginnerHTMLtext assignments withtextContent.Several places still use
innerHTMLto set loading/status text (e.g., lines 458, 502, 696, 860). Since these are plain text from i18n without HTML markup, usingtextContentwould be more consistent with the PR's objective.♻️ Suggested changes
- resultElement.innerHTML = debugI18n.t('fixing_rule_priority'); + resultElement.textContent = debugI18n.t('fixing_rule_priority');Apply similar changes to:
- Line 502:
resultElement.innerHTML = debugI18n.t('clearing_rules_reapply');- Line 696:
resultElement.innerHTML = debugI18n.t('collecting_diagnostics');- Line 860:
resultElement.innerHTML = debugI18n.t('getting_domain_rules');
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
debug-ui.jsshared/theme-init.jstoggle.css
✅ Files skipped from review due to trivial changes (1)
- toggle.css
🚧 Files skipped from review as they are similar to previous changes (1)
- shared/theme-init.js
🧰 Additional context used
🧬 Code graph analysis (1)
debug-ui.js (3)
popup.js (5)
fragment(147-147)fragment(233-233)result(225-225)result(300-300)result(332-332)shared/header-check-utils.js (1)
fragment(149-149)detect.js (13)
fragment(357-357)fragment(380-380)fragment(413-413)fragment(450-450)fragment(518-518)fragment(579-579)fragment(676-676)fragment(719-719)fragment(779-779)fragment(812-812)fragment(892-892)result(341-341)strong(361-361)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (4)
debug-ui.js (4)
156-158: Good fix: Separator now uses valid HTML structure.The
<hr>element has been correctly replaced with an<li>element using therule-separatorclass. This resolves the invalid HTML issue where<hr>was a direct child of<ul>.
96-161: LGTM: Well-structured DOM construction using DocumentFragment.The refactored code properly uses
DocumentFragmentfor batch DOM construction before appending to the live DOM. The use ofappend()for mixed text/element content (e.g., line 116) is correct and idiomatic. This approach minimizes reflows and improves rendering performance.
958-993: LGTM: Proper DOM-based table construction.The table structure is correctly built using DOM APIs with proper semantic elements (
thead,tbody,tr,th,td). The use of a singleDocumentFragmentfor batch insertion at line 993 is efficient and follows the pattern established elsewhere in the file.
1096-1182: LGTM: Clean implementation with proper DOM construction.The
testDomainCachefunction follows the established pattern of usingDocumentFragmentfor building UI before insertion. The null checks and error handling are appropriate.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Summary by cubic
全面改用原生 DOM API 构建 UI，替代字符串拼接，提升安全性与渲染性能。新增主题初始化脚本，按系统/本地偏好设置主题并修复主题闪烁。
Refactors
Bug Fixes
Written for commit 4f43266. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.