Require waitForElement optimizations for userscripts that run in project pages
Setting this issue as a reminder to research this and think of ideas. Most addons apply these optimizations but I will check if some of them don't.
We should also consider a re-thought of the waitForElement options API, which might be confusing for new contributors. Possibly {markAsSeen:true} should be the default?
Some suggestions:
- Remove the
markAsSeenoption, default it to true, and encourage the use ofaddon.tab.waitForElementOnce(doesn't exist yet) instead. - Avoid reusing the same set of
reduxEventseach time. Give a name to each set and expose them asaddon.tab.waitForElement.EDITOR_DOM_INITIALIZED, for example.
Regarding project pages:
- Each
waitForElementcall should indicate whether it plans to be executed in a project page (indicating true or false would be mandatory). - If above is true, then each call should indicate if it is intended for the project page, the project editor, or both. This replaces
isPlayerOnlyoptimizations. - Optimizations such as "currently in the code/costume/sound " (
editorTab.activeTabIndex) will continue to look the same, but maybe they could also be made more expressive. We should consider this, as there's 14 files that check if the tab index matches exactly a number.
Related comment: https://github.com/ScratchAddons/ScratchAddons/pull/6695#issuecomment-1734230346 - we could possibly kill the while(true) pattern in favor of events/callbacks of some sort
Related comment 2: https://github.com/ScratchAddons/ScratchAddons/issues/6880#issuecomment-1823304648 - careful with waitForElement:false, as an element might exist but possibly not all of its children. Probably a good idea to ignore an element unless all their children exist - unless if waitForElementOnce("body") ?
Related comment 3: https://github.com/ScratchAddons/ScratchAddons/issues/6956#issuecomment-1858406128 - worth running all selectors again if window has been resized
I'm thinking, maybe we could consider the editor and project pages completely separate concepts - and make it so that every userscript that wants to run on /projects/* needs to explicitly ask for it in the addon manifest. The "projects" match would cease to exist, and no amount of asterisks would be enough to run in project pages.
Also related:
If a waitForElement call includes reduxEvents, it should still run the selector for the first time, right? Looks like that is not the case, according to this addon: https://github.com/ScratchAddons/ScratchAddons/blob/master/addons/disable-auto-save/userscript.js#L10
Also related: If a waitForElement call includes
reduxEvents, it should still run the selector for the first time, right? Looks like that is not the case, according to this addon: https://github.com/ScratchAddons/ScratchAddons/blob/master/addons/disable-auto-save/userscript.js#L10
This code is wrong - if the addon is dynamically enabled, addListener is called twice.