erosman / support

Support Location for all my extensions
Mozilla Public License 2.0
170 stars 12 forks source link

Script doesn't work in Firemonkey #277

Closed rubaboo closed 3 years ago

rubaboo commented 3 years ago

Could you have a look at this script to see if it can be made to work with Firemonkey with some adjustments? It works in Tampermonkey… https://raw.githubusercontent.com/Wizmann/Utils/master/UserScript/WorkflowyPlus.js

erosman commented 3 years ago

Script uses JQuery functions without @require any JQuery.

I don't know which JQuery version it uses but try with JQeury 3 and see

// ==UserScript==
// @name         WorkflowyPlus
// @namespace    http://wizmann.tk
// @version      0.1
// @author       Wizmann
// @match        https://workflowy.com/*
// @grant        none
// @require      jquery-3
// ==/UserScript==

Unless..... the script is meant to be injected in the page content and use page JQuery. If the script works in TM, but not in FM|GM|VM, it would appear that TM injects scripts into the web page content.

rubaboo commented 3 years ago

Thanks, the script works with @require.

This is probably not very helpful, but if you want details: I did not check it with other M's. There are no instructions at https://github.com/Wizmann/Utils, but https://blog.workflowy.com/2016/01/06/inline-images/ suggests using TM for the script (without explaining why TM and not any other M).

rubaboo commented 3 years ago

Actually, I do have one more question. I have uBlock Origin installed, and this script still wouldn't work until I allowed Google Analytics: workflowy.com google-analytics.com * allow. Any idea why that is?

erosman commented 3 years ago

TM for the script (without explaining why TM and not any other M).

Tampermonkey • Documentation

If @grant is followed by 'none' the sandbox is disabled and the script will run directly at the page context. In this mode no GM_* function but the GM_info property will be available.

As I suspected, the script in TM runs in page context.

erosman commented 3 years ago

Tampermonkey • Documentation

If @grant is followed by 'none' the sandbox is disabled and the script will run directly at the page context. In this mode no GM_* function but the GM_info property will be available.

As I suspected, the script in TM runs in page context (which has security concerns).

If you want to mimic the TM and inject the script into page context, then this is what you can do ...

📌 GM.addScript() is FireMonkey only.

// ==UserScript==
// @name         WorkflowyPlus
// @namespace    http://wizmann.tk
// @version      0.1
// @author       Wizmann
// @match        https://workflowy.com/*
// @grant        none
// ==/UserScript==
/* jshint -W097 */
'use strict';

GM.addScript(`

function do_parseImg() {
    console.log("do_parseImg");
    $(this).nextAll(".content-img").remove();
    var lines = $(this).text().split("\n");
    var img_re = /^\!\[(?<property>.*?)(?:,(?<ratio>\d+))?\]\((?<url>.+)\)$/;

    for (var i = 0; i < lines.length; i++) {
        var line = lines[i].trim();
        var img = line.match(img_re);
        if (img === null) {
            continue;
        }
        var property = img.groups.property;
        var img_url = img.groups.url

        // console.log(property, img_url);

        if (property === "iframe") {
            $(this).after('<div class="content-img"><iframe width="100%" height="512" src="' + img_url + '" frameborder="0" allowfullscreen=""></iframe></div>');
        } else if (property === "audio") {
            $(this).after('<div class="content-img"><audio src="' + img_url + '" controls="controls"></audio></div>');
        } else {
            console.log(img);
            if (img.groups.ratio != undefined) {
                $(this).after('<div class="content-img"><img width="' + img.groups.ratio + '%" ' + 'src="' + img_url + '"/></div>');
            } else {
                $(this).after('<div class="content-img"><img src="' + img_url + '"/></div>');
            }
        }
    }
}

function parseImg() {
    console.log("parseImg");
    // console.log($("div.notes div.content"));
    // $("div.notes div.content").keyup(do_parseImg);
    // $("div.notes div.content").click(do_parseImg);
    $("div.notes div.content").each(do_parseImg);
};

$(window).bind("load hashchange", parseImg);
window.addEventListener('popstate', parseImg);

window.WFEventListener = event => {
  switch (event) {
    case 'documentReady':
    case 'expand':    // via click on + & Ctrl+Space on child only. NOT SUPPORTED: Ctrl+Down, Ctrl+Space on parent & menu
    case 'collapse':    // via click on - & Ctrl+Space on child only. NOT SUPPORTED: Ctrl+Up, Ctrl+Space on parent & menu
          parseImg(); break;
    case 'operation--move':    // moving any bullet, via mouse or KBS, including indent, outdent
    case 'operation--edit':    // automatic save after any editing text operations. Usually fires within 1s of last keystroke
          console.log(event);
          var node = WF.focusedItem().getElement();
          console.log($(node).find("div.notes div.content"));
          $(node).find("div.notes div.content").each(do_parseImg);
          break;
  }
};

var pushState = history.pushState;
history.pushState = function () {
    pushState.apply(history, arguments);
    parseImg();
};

// end of GM.addScript
`);
erosman commented 3 years ago

I am implementing a new feature to cater for this TM behaviour.

When testing, the above script still runs into error since https://workflowy.com/ doesn't have JQuery. Does it function fully in TM?

rubaboo commented 3 years ago

Yes, in TM it just works. I tried in a different profile though, one that doesn't have uBO. I can try with uBO next. Did you notice my question about Google Analytics? Took me a while to find what to unblock, GA was the last thing on my list to try unblocking.

rubaboo commented 3 years ago

@erosman I think I'm onto something. Tried TM+uBO, the script works, no need to unblock Google Analytics. That means FM is somehow requiring GA unblocked in order to work.

erosman commented 3 years ago

I get the same error in TM as in FM and that is JavaScript parsing error (Developer Tools Ctrl +Shift + j)

image

That is the issue with the script running. $ & $(this).nextAll etc are all JQuery function. If there is no JQuery in that page, there will be an error. I tested it on https://workflowy.com/

I cant test it in inner page as it requires login. It is possible that inner pages have JQuery. Is there a page that I can test on that doesn't require login?

Google Analytics shouldn't have any bearing on it as the script doesn't use GA.

rubaboo commented 3 years ago

I think we are discussing 2, if not 3 issues here. I can create separate issues for clarity if needed.

  1. The initial issue - script not working in FM. Fixed by adding // @require jquery-3 to the script. FIXED.
  2. You would like to add a new feature, to improve compatibility with TM scripts(?). This one is over my head. No comment.
  3. The FIXED (see #1) script does not produce a desired result unless I unblock GA in uBO. With TM, no unblocking required. I know GA should not have any bearing, but the issue is there and is repeatable. This is the issue I am currently concerned about. I will see about sharing an inner page with you where I can demo the issue. If I need to send you a secret URL, how can I do that?
erosman commented 3 years ago

Context

TM injects the script with no @grantinto page context while FM in the userscirpt context & GM|VM into content context.

The behaviour of the script will be different in different context. The new feature I am working will mimic TM if a new flag is added... that is for future.

JQuery

Above script requires JQuery to run but it doesn't have it. If the script works with TM without JQuery, that would mean the page that it is injected to by TM already has JQeury which the script can use.

In case of current FM, script can be injected in the page, like TM, by using the code given in https://github.com/erosman/support/issues/277#issuecomment-762837164

That way, there should be no need to add // @require jquery-3

uBlock Origin

I am guessing there are issues that I cant see from the front page that I tried.

Test URL

Do you visit https://chat.mozilla.org/? If so, you can send me a private message. You can find my name (even when I am not online) in https://chat.mozilla.org/#/room/#addon-reviewers:mozilla.org

rubaboo commented 3 years ago

Sent a DM.

erosman commented 3 years ago

I checked the sample page.

image

rubaboo commented 3 years ago

IDK, the injection way does not work for me at all. Since you don't have the same problem, I guess it's up to me to find out why I'm having this issue. Just one more thing - when you were testing, did you replace your uBO settings with the settings I uploaded to the chat? They are just "medium-mode" settings with some additional filters.

erosman commented 3 years ago

when you were testing, did you replace your uBO settings with the settings I uploaded to the chat? They are just "medium-mode" settings with some additional filters.

No... My uBO is almost the default setting. Try the default and see.

rubaboo commented 3 years ago

I give up. Tried everything. I don't have skills to find out where the problem is. Most puzzling is - in a brand new profile, with nothing but FM installed, the script does not work at all. Not with // @require jquery-3, nor injected. I'll leave the ticket to you to close or keep for your further research notes. (That's what I think some of your replies are - either notes to yourself, or you are greatly overestimating my proficiency with web programming.) Thanks for your help.

Oh, I tried the script with all 4 Monkeys. GM - "as is" logs an error, with @require no errors but not working. TM, VM - works as is, no @require or injection needed.

erosman commented 3 years ago

Oh, I tried the script with all 4 Monkeys. GM - "as is" logs an error, with @require no errors but not working. TM, VM - works as is, no @require or injection needed.

FM & GM inject into sandbox. FM injects into dedicated userScript sandbox. GM injects into content sandbox.

TM & VM inject into page. TM injects into page if @grant is none. VM injects into page if default injection mode is set to auto or page or @inject-into is set to page. image

The new release of FM will have a @inject-into option so you can set it to behave like TM & VM for this script. This feature is done.

I am in the middle of re-styling the options page to prepare for Android (it is a mess now). As soon as it is done, I will release the next version and you can test it.

Let's keep this open, as this is not the only script with page vs sandbox issue which results in some scripts working in TM|VM and not in FM|GM.

erosman commented 3 years ago

v2.13 with @inject-into support uploaded.

Try the script with it and let me know.

// ==UserScript==
// @name         WorkflowyPlus
// @namespace    http://wizmann.tk
// @version      0.1
// @author       Wizmann
// @match        https://workflowy.com/*
// @grant        none
// @inject-into  page
// ==/UserScript==

Testing on the demo link you provided:

Developer Tools (F12) -> Inspector

image

Developer Tools (F12) -> Console

image

Developer Tools (F12) -> Debugger

image

rubaboo commented 3 years ago

Yes, it works now. Thank you.

I don't feel this deserves an issue, and you might have this planned already - about the new layout of options page - it would be great if the sidebar was pinnable. In a desktop browser it makes sense to have it visible at all times, as before.

pintassilgo commented 3 years ago

@inject-into is a great improvement for compatibility, thank you. But is still required that we add it to scripts that need it. For scripts in active development, if the dev doesn't agree to add it, we're basically restricting FireMonkey to JS-skilled users, besides the hassle of having to fix scripts every time they update.

Violentmonkey uses "auto" injection mode by default, which means "always page, unless the script is blocked by CSP, in this case inject in content". https://violentmonkey.github.io/posts/inject-into-context/

What about adding a similar option? To be able to change the inject to content by default behavior.

Better... what about adding a way to set injection mode per-script? Outside of userscript code, with higher priority than @inject-into. So fixed scripts wouldn't break again when updated by devs that don't care about FM. Just like any other userscript manager allow users to set custom @include/@exclude without touching the code.

erosman commented 3 years ago

Yes, it works now. Thank you.

Great... :+1:

I don't feel this deserves an issue, and you might have this planned already - about the new layout of options page - it would be great if the sidebar was pinnable. In a desktop browser it makes sense to have it visible at all times, as before.

There are pro & cons with both set-ups. I had the sidebar panel at first since I liked to be able to quickly move through scripts. On the other hand, on smaller or 4:3 screens, the wider area for the code is also beneficial.

Let's see.... :thinking:

@inject-into is a great improvement for compatibility, thank you. But is still required that we add it to scripts that need it. For scripts in active development, if the dev doesn't agree to add it, we're basically restricting FireMonkey to JS-skilled users, besides the hassle of having to fix scripts every time they update.

What about adding a similar option? To be able to change the inject to content by default behavior.

@pintassilgo @inject-into is actually a metadata entry supported by ViolentMonkey (not supported by GM or TM).

Furthermore, as mentioned, CSP of the page can block script injection. The CSP has to be checked manually via headers. VM injects manually (read https://github.com/erosman/support/issues/274#issuecomment-765878053) which is not done in FireMonkey.

VM has 3 injection options which are page, content, auto.

what about adding a way to set injection mode per-script? Outside of userscript code, with higher priority than @inject-into. So fixed scripts wouldn't break again when updated by devs that don't care about FM. Just like any other userscript manager allow users to set custom @include/@exclude without touching the code.

That is possible but then bloats the preferences & code with a lot of new data. Adding @inject-into which is supported by FM|VM and ignored by GM|TM does not cause any hassle for the userscript developer. Doesn't that require just a tiny bit of effort? There is also the problem of GM functions which are supported by VM in page mode but not by FM & TM. We will get into the argument of, after setting page mode why GM functions doesn't work while it works in VM?

Let's see :thinking:

ATM, each userscript manger has features that are not compatible with others. Userscript developers would need to take that into account. FireMonkey user-base is still growing and there might be a time that userscript developers take notice.

pintassilgo commented 3 years ago

Thanks for detailed reply.

  • GM has no support for page mode.

Right. That's why I no longer use GM since v4.0. Incompatible with many scripts not written for it. I could fix scripts by myself, but when they update I'll need to fix them again and again... It doesn't sound right. I think of UserScripts as something almost universal, "cross-platform". My personal view is that UJS managers should try hard to preserve compatibility.

  • VM injects into page WITH GM functions which is a considerable security risk as web sites will also have access to those GM functions

Only if the UserScript exposes those functions to the page, right? Like with unsafeWindow.GM_getValue = GM_getValue or console.log(GM_getValue). Otherwise, even @grant'ing GM functions I can't access them with web console. If my reasoning is correct, the issue is in malicious/bad written UserScript. UserScripts have access to GM functions anyway, so I don't see much difference.

Adding @inject-into which is supported by FM|VM and ignored by GM|TM does not cause any hassle for the userscript developer. Doesn't that require just a tiny bit of effort?

I agree with you, but we both know that we can't expect that everyone would be willing to do this tiny effort. Violentmonkey has less than a tenth of users compared to Tampermonkey. Effort and costs may be a little higher, but I think UJS managers should try to cover those things, mainly those who lack higher userbase (that's why Firefox needs to implement Chromium web standards and not the other way around, as much as we don't like it).

But I perfectly understand that you think otherwise, fair enough, I'm just saying.

erosman commented 3 years ago

My personal view is that UJS managers should try hard to preserve compatibility.

Should userscript mangers adapt to various ways userscript developers like to write the code that may only work in one manger and not in others? :thinking:

In web design, it is the job of the web designer to develop content that works the same way in all browsers, not the other way round. Developing browser specific design has always been a sign of poor quality. :wink: The same principle applies to developing a userscript that only works in one userscript manager.

AFA compatibility, GM was the first and the pioneer. VM and TM did not preserve the compatibility in many aspect. TM & VM are not 100% compatible with each other either. Result is that some scripts work in one and not another.

Only if the UserScript exposes those functions to the page, right?

No... whatever is in page context is accessible by the page (web site). The whole point of different context is the separation of page & content & browser.

Like with unsafeWindow.GM_getValue = GM_getValue or console.log(GM_getValue). Otherwise, even @grant'ing GM functions I can't access them with web console.

web console has a different context which is content context. Furthermore, each content context is isolated. content JavaScript injected by one add-on cant access content JavaScript injected by another add-on. There can be many content context (i.e. sandboxes) in one page.

However, there is ONE page context which belongs to the web site. Whatever injected into that context, will also belong to the web site. That is the reason page XHR/fetch works fine in sending that site's credentials from page context but requires more parameters in GM functions.

From FireMonkey help ...

Extension JavaScript Context/Scope

  • Browser Context: Trusted privileged code to interact with browser
  • Content Context: Trusted extension JS injected into web page with API privileges
  • User-Script Context: Untrusted unverified 3rd party JS injected into web page without direct API privileges
  • Page Context: JS injected in a web page by the website

UserScripts have access to GM functions anyway, so I don't see much difference.

Userscripts have access to GM functions in userscript context & content context BUT websites i.e. page context, don't have access to them.

Note: content context is not as secure as userscript context. userscript context has only access to the GM functions set by the script manger. content context, in addition to GM functions set by the script manager, also has access to a number of browser APIs (read https://github.com/erosman/support/issues/276#issuecomment-761763943). Usersript managers can implement measures to combat this but there are possible to be bypassed.

Communicating with page context is done through a method that is also accessible to web site. Web site can then hijack this channel. It is a two way bridge.

But I perfectly understand that you think otherwise, fair enough, I'm just saying.

Feedback is good. I consider all feedbacks when developing. There are features now in FireMonkey that I personally wouldn't have included (and didn't at first). They were added after user feedback. :+1:

pintassilgo commented 3 years ago

I just noticed that FM's @inject-into page is different from VM.

VM doesn't append the script directly in the page. FM exposes everything. With FM, if my UserScript has let exposed = true and doesn't use IIFE, exposed will be accessible on the page . VM do it in a way that I can't find the script in source, while FM simply append a <script>.

erosman commented 3 years ago

VM do it in a way that I can't find the script in source, while FM simply append a Githubissues.

  • Githubissues is a development platform for aggregating issues.