-
Notifications
You must be signed in to change notification settings - Fork 0
yth8phyturchtgyutpqwuhitgyhwuqiop[kijuhgsiaoISJUHDFGJSUIO #12
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
Conversation
HGRUFIYGQegyecgyaescyigcgyuqwrcw];d41]5]23.\5[\2.]5[.1]5.[1]\26[.\]346.[]\34.[6
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe PR fixes incorrect Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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
🧹 Nitpick comments (1)
src/extension-support/extension-manager.js (1)
284-288: Remove commented-out code.The commented normalization logic is no longer used and adds clutter. Since
normalURLhas been replaced withextensionURLthroughout, this code can be safely removed.Apply this diff to remove the obsolete code:
- /* if (extensionURL.includes("ilelagal.site")) { - alert("Extensions using p*mod.site are deprecated, please swap them over to use forbiden mod.com instead.") */ - // so um since i "borrowed" this code from antartic bird mod theres a bunch of refernces and special cases to it.. so.... i dont care. im just removiing them all - - // const normalURL = extensionURL.replace(".site", ".com"); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/extension-support/extension-manager.js(3 hunks)
🔇 Additional comments (1)
src/extension-support/extension-manager.js (1)
294-295: Approve patchReplacing
normalURLwithextensionURLfixes the ReferenceError and bothgetSandboxModeandrewriteExtensionURLaccept raw URLs unchanged; no further changes required.
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: 0
🧹 Nitpick comments (1)
src/extension-support/extension-manager.js (1)
347-347: Duplicate detection fix acknowledged.The change preserves the original
extensionURLinpendingExtensions, which is then stored inworkerURLsat line 486. This ensures duplicate detection at line 275 works correctly for both sandboxed and unsandboxed extensions, addressing the concerns from the previous review.The
rewrittenURLproperty is currently unused (the worker receives the original URL viaallocateWorkerat line 487). If sandboxed extensions need to avoid re-fetching, consider passing the blob URL or rewrittenURL to the worker instead of the original extensionURL.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/extension-support/extension-manager.js(4 hunks)
🔇 Additional comments (2)
src/extension-support/extension-manager.js (2)
294-295: Critical fix: ReferenceError resolved.Correctly using
extensionURLparameter instead of the undefinednormalURLvariable. This directly resolves the "ReferenceError: normalURL is not defined" from issue TurboWarp#227.
319-319: Correct URL tracking for unsandboxed extensions.The change to store
extensionURL(the original URL) aligns with the duplicate detection logic at line 275 and fixes the ReferenceError for unsandboxed extensions.
HGRUFIYGQegyecgyaescyigcgyuqwrcw];d41]5]23.\5[\2.]5[.1]5.[1]\26[.]346.[]\34.[6
Resolves OmniBlocks/scratch-gui#227