-
-
Notifications
You must be signed in to change notification settings - Fork 524
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
Better Clear Recent History Dialog #2272
Conversation
Better Clear Recent History Dialog Signed-off-by: Gunir <[email protected]>
📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to the configuration settings within the Changes
Possibly related PRs
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
src/browser/app/profile/zen-browser.js (1)
Line range hint
243-249
: Update PR description to include all changes.While the change to the Clear Recent History Dialog aligns with the PR objectives, the addition of Kyber-related experimental features was not mentioned in the PR description. Consider updating the PR description to include:
- The rationale for enabling the old Clear Recent History Dialog.
- An explanation of the Kyber-related changes, their potential benefits, and their experimental nature.
- Any known risks or considerations for enabling these experimental features.
This will provide better context for reviewers and future reference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/browser/app/profile/zen-browser.js (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/browser/app/profile/zen-browser.js (2)
Line range hint
249-249
: Group Kyber-related preferences and provide context.This preference is closely related to the previous Kyber TLS preference. Consider grouping them together and adding a comment explaining the relationship between Kyber in TLS and HTTP/3.
Please address the following:
- Are there any interdependencies between the TLS and HTTP/3 Kyber implementations?
- Has this been tested in conjunction with the TLS implementation?
- Are there any known issues or limitations when enabling Kyber for both TLS and HTTP/3?
#!/bin/bash # Check for any existing HTTP/3 Kyber-related code or comments rg -i "http.?3.*kyber" --type js
248-248
: Verify the implications of enabling Kyber in TLS.While enabling Kyber, a post-quantum cryptography algorithm, could enhance future security, it's important to ensure this experimental feature has been thoroughly tested and is ready for general use.
Please provide more information on:
- The current state of Kyber implementation in TLS.
- Any performance implications or compatibility issues.
- Whether this should be enabled by default or made configurable by users.
Better Clear Recent History Dialog
Clearing Offline Website Data is useful, because Zen will get slower as it accumulates more Offline Website Data (localStorage and indexedDB), clearing this returns back good performance, and there's websites that spam Offline Website Data to GBs, causing slows down to Zen so this is good for maintaining Zen for performance.
Summary by CodeRabbit