More modular script system

[roxwize]

With how disjointed the scripts can be in their implementations, the amount of files for each new feature is bound to get ridiculously large rather quickly. I think it'd be a lot wiser to implement a more modular system that allows importing of various utility functions and is scoped (#19), along with any other features that might be convenient. Right now, there are a lot of repeated tasks (like adding a script tag to the page) that could use helper functions or similar; I'm up to suggest any potential additions to make the codebase a lot more manageable, approachable, and extensible (these are all shoddy buzzwords but bear with me), and I'll see if I can come up with any ideas later to make this work somehow. I think it'd be best to do this as early as possible so that so much cleanup won't be required as this does have the opportunity to be a breaking change.

[aprzn123]

I agree. Once we merge these two open PRs, we should hold off on accepting any other new features until we deal with this

[aprzn123]

Everything is merged, so it's time to think about this some more.

[roxwize]

I think having a sort-of global thc object (choosing that name since 3c would be invalid) that has helper functions such as fetching user ID, adding script element to a page, and other programmer-friendly things would be a good start. I think that a good point to begin from would be to sort of define APIs or the likes for scripts to use; so there could be a tcas API for handling the website itself, thc for handling extension-side stuff, and some other namespaces. But, if that's overkill, we could just opt for a single thc namespace containing helper functions unless the road ahead dictates that these won't be necessary.

In any case, I think creating a roadmap for everything we need to do to make this work would be a good way to kick things off. Since this is inherently a very breaking change, I feel like we should lay out all of the planned changes in order of when we plan to implement them, then check them off a list. So for example, start with making extension applications more portable and modular (could be split into multiple subtasks) and then branch into adding scoping and global helper functions. This is the kind of thing that requires more than a few minutes of thought so it might take a very long time to get this fully fleshed out, but I think it's definitely possible.

[aprzn123]

Here's my thoughts for the first couple steps, at least:

1. Write a function that take the path to a script and injects it into the page
2. Adapt all the features to use that function
3. Design and write utility functions for reading/saving the config
4. implement those throughout the codebase
5. ???

[roxwize]

6. Profit.

[roxwize]

I'll go ahead and start writing the global functions library. Do you think that'd be a good first step?

[Kylljoy]

Here's my thoughts for the first couple steps, at least:

1. Write a function that take the path to a script and injects it into the page

Mmmm.... I know everything would have to be approved to be in the extension repo, but it still rubs me the wrong way that you're injecting code with little sandboxing. That's raising some red flags.

[Kylljoy]

While it would be generally less flexible, perhaps a rudamentary system for pre-built functions, and piping/chaining would be better? You'd obviously need a dedicated tokenizer/lexer with custom grammar and a built-in interpreter (or dedicated embedded VM for custom architecture bytecode), but it would skirt sandboxing by providing limited access. And if you take piping/grammar cues from BASH, it probably wouldn't be that bad.

I promise this isn't a poorly-disguised pitch for using an embedded ASH system (especially since that's still in infancy), but in the interest of semi-professionalism, sandboxing "untrusted" code is always the good option.

[aprzn123]

@thekifake: would this be the ES module system, and is it legal to do import whatever from browser.runtime.getURL("some_path")?

@Kylljoy: I'm not sure about that, but it is definitely worth looking into minimizing the amount of code that is injected. The problem is mostly just that sometimes we need to access and modify things based on objects that only exist within the scope of the page.

[Kylljoy]

I wouldn't say it would be trivial, but we're limited the scope massively by restricting this to a site-altering system, right? Theoretically, building a custom scripting system wouldn't be too difficult if you restrict the grammar a tad. Making the tokens limited to the magnitude of a couple tens (basic assignment, arithmetic, control flow), and making it strongly typed would massively reduce the complexity of the lexing/parse system. It would be relatively simple to build a basic parser, especially considering nothing here technically has to be Turing complete?

A simple grammar akin to bash or VBasic along the lines of

var1=3CanDefinedFunctionA(arg 1, arg 2)
if (state)
  simpleInjectionSubroutine(tagtype or id, $var1)
endif
etc.

would be relatively feasible to include. Even though my familiarity with JS is admittedly.... limited.... I could probably throw something together.

And yes, I guess I am involving myself with this project even though I tried to avoid getting distracted from my own shit.

[Kylljoy]

Although this may be me seeking to make a project more technically challenging and thereby more gratifying. As usual.

[roxwize]

I think that is a bit too overcomplicated considering the scope of this project. It's a simple extension that is trying to be as vanilla as possible from what I can tell, so that seems far too overkill in the grand scheme of things. However, sandboxing is definitely something that has to be addressed somehow.

[aprzn123]

Spent some time DMing with Kyle about this, and it's so tempting to try but I think ultimately it's not worth it.

[roxwize]

Think I should start working on global functions or try to think about getting sandboxing working properly first?

[aprzn123]

I'm thinking that it's not worth going full in on sandboxing, but we should probably have some process where we have a couple people look through PRs before we merge them to check for anything potentially vulnerable or malicious. So probably get started on the functions then.

[aprzn123]

When we implement the config save/load tooling, we should implement imports and exports as first-class functionality, as described in #30 here.

[HumanoidSandvichDispenser]

I'm thinking that it's not worth going full in on sandboxing, but we should probably have some process where we have a couple people look through PRs before we merge them to check for anything potentially vulnerable or malicious. So probably get started on the functions then.

I think we should also have a guideline where injected scripts can not make requests with fetch. If it does, it must do so through a specific util or helper function for their use case.

For example, the fetch call in the skipSavedQuestionButton.js script should be replaced with a skipQuestion util function that skips questions.

https://github.com/aprzn123/TheThirdCan/blob/bb2c24f9010c3d7c1749cb93d8336a8a8e67028d/src/injected/skipSavedQuestionButton.js#L39-L42

[roxwize]

ES6 imports are able to work in the internal extension scripts without any errors. Here is a shortened version of answer.js:

import thc from "../resource/thc.js";

function enableSkipConfirm(always) {
  thc.InjectScript(always ? "injected/alwaysSkipConfirm.js" : "injected/textSkipConfirm.js");
}

function enableSkipForNow() {
  thc.InjectScript("injected/skipForNowButton.js")
}

It's certainly not rocket science, but it's a start, and it means that global functions shouldn't be difficult at all to implement. Any suggestions for global functions that are needed?

[aprzn123]

ES6 imports are able to work in the internal extension scripts without any errors

Ok, that's actually a bit of a surprise. Not going to complain, though!

One suggestion for injectScript would be to automatically prepend "injected/", since we only want to allow script injection from that directory.

[HumanoidSandvichDispenser]

I'm curious where the H comes from in thc. Is it supposed to be THird Can? Or tetrahydrocannabinol? Any way I wouldn't object to it.

[aprzn123]

THird Can, yeah. TTC might be better, but it doesn't really matter at all

[aprzn123]

Honestly, I might recommend changing it to ttc or t3c, if only to make sure that mozilla and chrome reviewers don't think it's weed related and ban us

[roxwize]

I changed the name of the module to third since that makes a lot more sense and doesn't really sacrifice anything.

I feel like the module should be imported in a way that doesn't need it to be reimported in every single internal file. Also, I lied. I forgot to refresh the extension. It DOES give errors. But the dynamic import() function should work with the extension if Firefox won't get mad. I just need a way to make it so that third can be imported once, and then used across the entire extension. I figure turning the various features of the extension into functions that take third as an argument and are ran when the document is loaded could totally work for that, but I'm not sure how I'd do it for every other file. Honestly, I'm kind of stumped. And tired. Ideas?

[aprzn123]

I just need a way to make it so that third can be imported once, and then used across the entire extension.

I'm not sure if that's possible? Each script runs in separate contexts. Can you give an example for what you mean about turning features into functions that take third?

[roxwize]

Hm. I'm a bit tired, so I'd rather come back to that first part tomorrow. I'm still trying to figure out how to import third in a way that doesn't require a bundler (and I hate bundlers, by the way). But as for the latter half of your comment, I mean wrapping the code that runs a function (such as adding the "skip for now" button) in a function with a single argument passed into it, third. The internal script then goes through its cycle checking what scripts need to be ran and enabled, and when it finds one, it runs it. Think of it as this:

  let results = fetch(third.GetPath("default_settings.json"))
    .then((response) => response.json())
    .then((settings) => browser.storage.sync.get(settings));
  results.then((cfg) => {
    if (cfg.skipConfirm) {enableSkipConfirm(third, cfg.skipConfirm === 2);}
    if (cfg.skipForNow) {
      enableSkipForNow(third);
    }
  });

but except, instead of the functions adding a script tag to the page that lets their functionality be injected onto the page, running the function just runs its changes directly with preferably only one or two script tags required. I have no idea how this might work and I am ridiculously tired so this might be completely infeasible, but I feel like it's sort of what I've been trying to get at.

[aprzn123]

I agree about bundlers, one of the things that I am holding steadfast on for this project is that there should be absolutely no build step, no node_modules, none of that. As for importing, it looks like we just need to use dynamic import. We can address the rest of that in the morning; script tags are mostly only necessary when we need to access a page-local variable (like TC)

[roxwize]

I've been trying that for a while now, and I still can't figure it out. I'm able to get the script imported, but I can't do anything with it, and logging it to the console just gives me this:

(async() => {
  const src = chrome.runtime.getURL("resource/third.js");
  const third = await import(src);
  console.log(third)
  let results = fetch(third.GetPath("default_settings.json"))
    .then((response) => response.json())
    .then((settings) => browser.storage.sync.get(settings));
  results.then((cfg) => {
    if (cfg.skipConfirm) {enableSkipConfirm(cfg.skipConfirm === 2);}
    if (cfg.skipForNow) {
      enableSkipForNow();
    }
  });
})();

[aprzn123]

Can you push your changes to your fork so I can take a look?

[roxwize]

Sure.

[roxwize]

Done

[HumanoidSandvichDispenser]

It's accessible through the default property.

If you do const third = (await import(src)).default; it should work

[roxwize]

That works! Thank you. My question now is how would I pass third into a module without having to import it within the file itself? Hopefully doing this will make it so that the number of script tags required should only be one or two, as I'm planning to just have there be a script file attached that just runs a bunch of functions representing the code of each module; third would be defined within it, and it would pass third into the functions like this:

// (Hopefully) only injected script

const third = { /* ... */ }

(function(third) {
  let textbox = document.getElementById("answer_text");
  let purpleButton = document.querySelector("button.purple");
  let buttonArea = purpleButton.parentElement;
  let skipForNowButton = document.createElement("button");
  skipForNowButton.innerText = "Skip for now";
  buttonArea.insertBefore(skipForNowButton, purpleButton.nextSibling);

  // hacky way to get user id
  const attr = document.querySelector("body").getAttribute("onload");
  const userID = Number(attr.substring(18, attr.length - 1));

  function shouldSkip() {
    const cfgStr = window.sessionStorage.getItem("ttcConfigState");
    const cfg = JSON.parse(cfgStr);
    if (cfg.skipConfirm !== undefined) {
      // check if user enabled skip confirmation. if so, prompt for confirmation
      // if necessary according to the user's settings
      if ((cfg.skipConfirm === 1 && textbox.value !== "") || cfg.skipConfirm === 2) {
        return confirm("Are you sure you want to skip this question for now?");
      }
    }
    return true;
  }

  skipForNowButton.onclick = () => {
    if (shouldSkip()) {
      // the best way to get a new question would be to reinit the QA
      TC.QA.Answer.init(userID);
      textbox.value = "";
    }
  };
})(third);

[aprzn123]

Why not just import third in the same way? It should be accessible still.

[roxwize]

Yes, but I want to make functions easier to get started writing (I don't want scripters to have to constantly copy and paste anything) and I want to reduce load times from having to import the module over and over.

[aprzn123]

I get what you're saying, but the performance impact should be negligible (especially for an extension this small). The thing with making an async function is a bit annoying, but I think if we inject all our script tags as <script src="whatever" async>, we don't need that and can just await import() directly. I'd have to try it out, but I suspect that is the case.

[roxwize]

I'm tired, again, meaning I'm out of my programming mood. I have a rough draft of what to do, but it isn't working. Importing third just seems to not want to work after everything I've tried. I've pushed it to the fork. If you want to, see if you can do anything with it.

Also, I finished the Alert function. I think it could be handy.

[aprzn123]

Maybe you can PR into a separate branch that we can all work on? I want to see if I can do anything with this

[roxwize]

Alright, sounds good to me. Check #32.

[aprzn123]

So, I'm thinking about not actually importing third to injected scripts, and instead having the following system:

The current API would have us doing this:

if (cfg.configOptionOne) {
  third.InjectScript("something");
}
if (cfg.configOptionTwo) {
  third.InjectScript("somethingElse");
}

I think that instead, we should have a utility function (exact API not necessarily fully pinned down yet) something like this:

third.InjectToggleableScripts({
  "configOptionOne": "something", 
  "configOptionTwo": "somethingElse"
}, "pageName");

In order to incorporate the options variable, we could make the "something" and "somethingElse" optionally objects.

Then, inside of injectToggleableScripts, (assuming at least one script is enabled) we first inject some sort of polyfilled utility (e.g. injecting a userID variable that is defined differently depending on what page this is on).

[aprzn123]

So, what utilities do we want to make available for injected scripts? Definitely userID and ttcConfigState, but what else?