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

better plugin wrapper #358

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

johnd0e
Copy link
Contributor

@johnd0e johnd0e commented Mar 7, 2020

Use unsafeWindow, thus no need to 'inject script in site context' anymore.

TODO:

  • Make sure that mobile app is ready
  • More tests needed with misc plugins, testing different apis (such as XHR)
    https://web.archive.org/web/20141008055234/http://www.oreillynet.com/lpt/a/6257
  • IITC Button is not ready yet: Expose unsafeWindow to plugins IITC-Button#32
  • May be we need some markers in code, to define exact places of different wrapper parts.
    That can be used for easy updating of embedded wrapper.
  • May be we need some additional property to pass additional data from userscript manager
  • May be pick another name for plugin_info
  • It's possible to keep reference to original sandboxed window.
    But this also can be done manually, so probably not
  • May be it's time to introduce setup arguments.
    We could pass iitc api as argument. And may be something other, e.g. logger object
  • Some more improvements pending (see below)

This change is Reviewable

Use `unsafeWindow`, thus no need to 'inject script in site context' anymore.
- wrapper is simpler now
- no need to cut it out in mobile app
  (but we keep that code for all legacy plugins)
- whole `scriptMetaStr` is added to plugin_info, to be able to analyse more properties
- it's easier to debug plugins now
  To set breakpoints switch to Web Developer Tools' Debugger/Sources tab
  and find related script in Tampermonkey's list of `userscript.html?id=<guid>`
- it's possible to use GM_* functions directly (not in mobile app, for now)
  - https://wiki.greasespot.net/Greasemonkey_Manual:API
  - https://www.tampermonkey.net/documentation.php
@johnd0e johnd0e added core development general development issue labels Mar 7, 2020
@johnd0e johnd0e linked an issue Mar 7, 2020 that may be closed by this pull request
}
setup.info = plugin_info;
(window.bootPlugins = window.bootPlugins || []).push(setup);
if (window.iitcLoaded) { setup(); }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps better expose this:

function safeSetup (setup) {

Otherwise we would need special handling to see such plugins in About:

window.aboutIITC = function () {

Comment on lines +12 to +17
// PLUGIN AUTHORS: writing a plugin outside of the IITC build environment? if so, delete these lines!!
// (leaving them in place might break the 'About IITC' page or break update checks)
plugin_info.buildName = '@build_name@';
plugin_info.dateTimeVersion = '@build_date@';
plugin_info.pluginId = '@plugin_id@';
//END PLUGIN AUTHORS NOTE
// END PLUGIN AUTHORS NOTE
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid this, if we add this info to meta block, see #242.

pluginwrapper.py Outdated
function wrapper(plugin_info) {
// ensure plugin framework is there, even if iitc is not yet loaded
if(typeof window.plugin !== 'function') window.plugin = function() {};
var plugin_info = (typeof GM_info === 'undefined') ? {} : (function (s) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we should rely on GM_info definition, but iOS is not ready: HubertZhang/IITC-Mobile#1

@johnd0e
Copy link
Contributor Author

johnd0e commented Mar 10, 2020

Unfortunately we still cannot avoid script injecting for Greasemonkey (and IITC-Button).
As appeared, such code is just ignored:

window = ...

It'd be possible to replace it with local variable, but it's not enough for good compatibility with all existing 3rd-party plugins: we also need to replace global.

I do not know it it ever possible.
I've tried to apply with (unsafeWindow) statement, but then we get other issues (Error: Permission denied to access property [...])

So for Greasemonkey I have to restore previous method involving script injection.

@johnd0e johnd0e added the WIP Work in progress || Proof of the concept label Mar 10, 2020
@modos189 modos189 force-pushed the master branch 3 times, most recently from a34b800 to 7b9edd5 Compare December 12, 2020 07:21
@le-jeu
Copy link
Contributor

le-jeu commented Oct 27, 2021

* Direct script execution make debug easier:
  To set breakpoints switch to Web Developer Tools' Debugger/Sources tab
  and find related script in Tampermonkey's list of `userscript.html?id=<guid>`

I tried to use sourceURL to name script and make them appear in the source tree

// ...
script.appendChild(document.createTextNode('('+ wrapper +')('+JSON.stringify(info)+');'));
if (info.script && info.script.name)
  script.appendChild(document.createTextNode(`//# sourceURL=${encodeURIComponent(info.script.name)}`));
(document.body || document.head || document.documentElement).appendChild(script);

I guess this is not compatible with IITC Mobile but the inserted script is correctly listed in source (from intel.ingress.com) (firefox: violent/grease monkey)

I used info.script.name but this can also be done by the build system
Sadly, the //# has to be added before the script is inserted in the DOM (wtr to iitc mobile)

Edit: Possible workaround for mobile

// if on last IITC mobile, will be replaced by wrapper(info)
var mobile = `script.appendChild(document.createTextNode('('+ wrapper +')('+JSON.stringify(info)+');'));
(document.body || document.head || document.documentElement).appendChild(script);`;

if (mobile.startsWith('script')) {
  script.appendChild(document.createTextNode('('+ wrapper +')('+JSON.stringify(info)+');'));
  if (info.script && info.script.name)
    script.appendChild(document.createTextNode('//# sourceURL=' + encodeURIComponent(info.script.name)));
  (document.body || document.head || document.documentElement).appendChild(script);
} else {
  // mobile string
  wrapper(info);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core development general development issue WIP Work in progress || Proof of the concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants