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

Fix multiple pinned ingress tabs #14

Merged
merged 4 commits into from
Jul 9, 2019
Merged

Fix multiple pinned ingress tabs #14

merged 4 commits into from
Jul 9, 2019

Conversation

modos189
Copy link
Contributor

@modos189 modos189 commented Jul 4, 2019

And fix multiple pinned ingress tabs #11

It's necessary to thoroughly test

@modos189
Copy link
Contributor Author

modos189 commented Jul 4, 2019

@johnd0e what you thing about it?

@johnd0e
Copy link

johnd0e commented Jul 4, 2019

Inject UserScripts to context of page
And fix multiple pinned ingress tabs #11

Is these 2 features connected?

what you thing about it?

TM-way in general is not good, as it grants 'full access' by default, which often leads to untidy development style.
I believe that IITC plugins can be written without injections, in 'fair' way, and it's just a matter of changes in wrapper.

Plugins actually do not need independent execution, as all we need from plugin - it's setup function, which will be executed by the main IITC script in it's context.

But to get pure plugin code IITC-Button has to cut out wrapper (like IITCm does).

I am not sure that this my answer is relevant to your question, may be you tell why do you want to mimic tampermonkey?

@modos189
Copy link
Contributor Author

modos189 commented Jul 4, 2019

Is these 2 features connected?

Yes. Previously, plugins were inject directly from extension, which made it difficult to support multiple tabs. Now extension creates an event to inject plugin, and each tab inject the plugin, if it is not already enabled.

It's important that both old plugin injection and new page event capture, both in context of an extension. But old tabs.executeScript created a context for each plugin, and new way has one context. This allows to get rid of fix window.plugin.missions = true;

But plugins still don't have access to window.PLAYER, so I inject them into the context of page to get rid of code to retrieve user data.

I can save function to retrieve user data and run plugins in context of an extension. Use a new injection method for plugins based on a page event, but not run in the page context.

So, the choice between the possible untidy development style and an one ugly function to retrieve user data. Now I'm more inclined to the second option.

But to get pure plugin code IITC-Button has to cut out wrapper (like IITCm does).

Unfortunately, there are popular plugins that use custom wrappers and break when removing wrapper.

@johnd0e
Copy link

johnd0e commented Jul 4, 2019

But old tabs.executeScript created a context for each plugin, and new way has one context. This allows to get rid of fix window.plugin.missions = true;

We already discussed ingressmosaic plugin, and it's wrapper is broken anyway, so I do not think that workaround with window.plugin.missions = true; was so bad.

But plugins still don't have access to window.PLAYER, so I inject them into the context of page to get rid of code to retrieve user data.

Again, this single workaround is not worse than injecting of EVERY plugin.

Unfortunately, there are popular plugins that use custom wrappers and break when removing wrapper.

They break because their custom wrapper removed incorrectly. But it's possible to use special processing (depending on plugin name).

To summarize:

  1. injecting scripts is easy and compatible with any plugin.
  2. not injecting is more tricky, and less compatible (require custom workaround for some plugins)
    but potentially more proper.
    It will be easier when IITC-CE get more adoption: we can influence plugin authors to use proper wrapper.

I'd prefer 2nd approach.
But, considering the 1st is easier, you can implement it for now, and think about 2nd later (maybe)).

@modos189 modos189 changed the title Inject UserScripts to context of page, like Tampermonkey. Fix multiple pinned ingress tabs Jul 5, 2019
@modos189 modos189 merged commit a27d8fc into master Jul 9, 2019
@modos189 modos189 deleted the context branch July 9, 2019 12:00
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.

2 participants