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

No-editor, no-eval dev branch #9

Closed
wants to merge 10 commits into from
Closed

No-editor, no-eval dev branch #9

wants to merge 10 commits into from

Conversation

marcustyphoon
Copy link
Owner

@marcustyphoon marcustyphoon commented Dec 7, 2024

The XKit Editor, which lets users customize the code of their scripts from a special page in the browser, is built on the fact that New XKit stores its executable extension code in storage and runs it using eval. This is how user script extensions work, and as I understand it is tied to the history of this extension as a collection of user scripts and/or as a user script manager specifically for Tumblr.

This is kind of neat, and doesn't seem to break any Firefox addon policies per my line-by-line reading... and, I mean, violentmonkey is on the store, right? On the other hand, I have to assume that if we had metrics they would report that the editor functionality is basically completely unused by our userbase. The core functionality of the extension would still inarguably exist without the editor, and maybe a reviewer would argue that that's not a valid use of eval. And review aside, I think there are valid arguments to be made on both sides of whether the editor version or the no-editor version of the codebase is better. (It does kind of involve deciding on the purpose of this codebase going forward, which is another discussion.)

Anyway, I've never been interested in having that decision hinge entirely upon whether one version of the codebase exists and functions or not; I had the technical side solved ages ago. Thus, here is a fully functional branch with the XKit editor's save functionality disabled, with no eval use or storage of extension code and which thus has no version-number-based internal update functionality because the latest versions of scripts are always run directly out of the package (internal update notifications are gone, though we could put them back). This includes cleaning up all remaining web-ext lint warnings by factoring or killing some code in outdated scripts.

Discuss!

Technical notes:

  • This includes the commits of Update browser requirements to es2022 new-xkit/XKit#2176.
  • XKit's internals expect script metadata to be accessible synchronously, but dynamic import (like extension storage access) is asynchronous. XKit Bridge makes storage contents synchronously accessible by keeping them in a global variable and delaying initialization while populating it to solve this; I had to create XKit.installed.data and XKit.installed.init() to serve a similar purpose.
  • install_extension() in xkit.js was left as-is in my 7.10.0 migration to keep the API surface of what used to be "the JSON downloaded from XKit servers/Github Pages" stable, but there's no point to doing that anymore and it's awful. It's implemented as tweaks to the defaults in loadExtensionData() here.

The rest of the diff should be comprehensible from the above points.

@marcustyphoon marcustyphoon changed the title no-eval dev branch No-editor, no-eval dev branch Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant