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

Content-scripts are not injected correctly #7597

Closed
panther7 opened this issue Oct 16, 2020 · 29 comments
Closed

Content-scripts are not injected correctly #7597

panther7 opened this issue Oct 16, 2020 · 29 comments
Assignees

Comments

@panther7
Copy link

panther7 commented Oct 16, 2020

NWJS Version: 0.49.x, 0.50.x
Operating System: Win 10

Expected behavior

Insert content-scripts correctly, works fine in NWjs 0.47.3 and NWjs 0.48.4

Actual behavior

Sometimes, after init/load webview and injection content-scripts (via webview.addContentScripts).

How to reproduce

  1. Example.zip
    • contentscript.js - injected to webview, easy example for chrome.runtime communictaion, example showing command message hello-world from webview to window.
      connection.command('hello-world', null, (returnData) => console.log(returnData));
    • window2.js - chrome.runtime.onMessage, return to webview 'Hi!', if receive hello-word message.
  2. Run with NWjs, check devtools (shown automatically)
  3. Some startup, incialized wrong, missing contentscript.js
    2020-10-16 14 21 43
  4. Some startup, incialized ok
    2020-10-16 14 22 33
@panther7
Copy link
Author

@rogerwang Could you help me with? Or can i help you with this issue?

@panther7
Copy link
Author

panther7 commented Oct 27, 2020

This is blocker for our project. Is anything, which i can help resolve with this issue?

@rogerwang

@panther7
Copy link
Author

panther7 commented Nov 1, 2020

Still broken in v0.49.2

@rogerwang rogerwang self-assigned this Nov 11, 2020
@rogerwang
Copy link
Member

Hello. It works for me -- here is what I got after I run your sample in 0.49.2:

1

@panther7
Copy link
Author

Try run it several time please, it is works to 50%. It is strange.

@rogerwang
Copy link
Member

I tried it for 10 times and it always works...

@panther7
Copy link
Author

panther7 commented Nov 12, 2020

It is very strange, now i tested several times. Recorded to gif.

aaa
video

Btw, dev tools are narrower and narrower after every startup.

@TheJaredWilcurt
Copy link
Member

Reproduction Attempt Process:

  1. On Windows 10 64-Bit
  2. Download the latest NW.js SDK (0.49.2 64-Bit)
  3. Download provided example zip
  4. extract both
  5. Put files from example into a package.nw folder next to nw.exe
  6. Open task manager
  7. Run NW.js > Application launches > Console shows correct output > close NW.js
  8. Wait for all nw.exe process to be removed from the list of running processes in Task Manager
  9. Repeat the last two steps 10 times

My results are the same as Roger's. Cannot reproduce.

@bluthen
Copy link
Contributor

bluthen commented Nov 12, 2020

Maybe race condition and dependent on the hardware or system load? I usually get around these type of issues by finding right spot to split the task with a timeout. Hate doing it but seems unavoidable sometimes.

@panther7
Copy link
Author

panther7 commented Nov 13, 2020

I tested on 3 differents PC and all has same issue. In NWjs 0.47.3 and below I never seen this issue (v0.48 was skipped works in 0.48.4 too).

@corwin-of-amber
Copy link
Contributor

@panther7 Also could not reproduce, but I would suggest deferring the creation of the Connection object until the page has loaded; i.e. instead of:

const connection = new Connection();

which occurs at initialization time, you may try:

document.addEventListener('DOMContentLoaded', () => {
    window.connection = new Connection();
});

Or any other event that is appropriate, e.g. 'load' or 'readystatechange'.

@panther7
Copy link
Author

panther7 commented Nov 13, 2020

@corwin-of-amber Thanks for suggest, but it still is not fixed.

2020-11-13 15 52 40

or sometimes

2020-11-13 15 56 19

I hate these random issues.

@corwin-of-amber
Copy link
Contributor

Actually @TheJaredWilcurt, I have tested it a bit more (on macOS) and indeed when I reload the window (that is, Ctrl+R in the devtools window) rather than quit & relaunch, the generated_script_file (the one that holds params) is missing about one out of three times.

When quitting & relaunching, it is always there.

@panther7
Copy link
Author

@TheJaredWilcurt Still cant-reproduce? Last test from @corwin-of-amber confirmed my issue.

Btw, I tested today in Chromium v86.0.4240.198 and it works properly.

@panther7
Copy link
Author

Tested on latest nwjs0.50, still broken.

@corwin-of-amber
Copy link
Contributor

@panther7 it does seem like the only missing thing is params. Your content script is loaded correctly in all the reloads. Can you perhaps work around this by using something else instead of params.extensionId and params.frameId? E.g. you can generate id's in the main page and send them to the webview via postMessage.

@panther7
Copy link
Author

panther7 commented Nov 15, 2020

@corwin-of-amber hmm, it seem, that nwjs after v0.47 has more "async" loading content-scripts.
Now, i tested merge array content-scripts to one and seems works:

js: {
     code: `var params = { test: true, frameId: '${uuid}', extensionId: '${location.hostname}' };`,
     files: ['contentscript.js']
},

// Edit: Works in this simple demo, but in our huge project not. :-/

@panther7
Copy link
Author

panther7 commented Nov 18, 2020

Sometimes, i have this error:

Uncaught TypeError: Error in invocation of runtime.connect(optional string extensionId, optional object connectInfo): chrome.runtime.connect() called from a webpage must specify an Extension ID (string) for its first argument.

"String extensionId is optional", but error say, that must be present... there is something broken in NWjs.

screenshot

page.js is content script injected into webview.

@panther7
Copy link
Author

panther7 commented Nov 18, 2020

I created new example, it is more simple. It use only chrome.runtime.sendMessage and chrome.runtime.onMessage. Example opens 20 webviews and trying communication, if something fails, then you shows error alert.

Test in NWjs:

Please, try it.
@rogerwang @TheJaredWilcurt @corwin-of-amber

Thank you very much.

@rogerwang
Copy link
Member

@panther7 I don't think you are using the chrome API correctly -- as error message reported from your example, it prompts that you are missing a parameter 'extension id'. Please check https://developer.chrome.com/extensions/runtime#method-sendMessage

@panther7
Copy link
Author

panther7 commented Nov 24, 2020

@rogerwang Thanks for response.

What is wrong? API is same over years. Works correctly up to NWjs 0.48. Param extensionId is optional. Last example is simply extreme and illustrates, that NWjs sometimes "calls/includes/execs/whatever" wrong with chrome.runtime API.

// Edit: Here is example with extensionId. The extensionId is included via "second" object with name params.

@rogerwang
Copy link
Member

According to the doc, the param is required when the API is called from web pages, that is what the sample is doing from a webview. So it's expected to fail.

It may "work" for you in previous versions, but that was a bug and is fixed now. Please do not rely on it.

@panther7
Copy link
Author

panther7 commented Nov 24, 2020

@rogerwang chrome.runtime and extensionId are not truly issue.

Next example is without them. Only define content-script code with variable params. The contentscripts.js trying read params.extensionId, like:

window.js:

webview.addContentScripts([
    {
        js: {
            code: `
                var params = {};
                params.extensionId = '${location.hostname}';
            `,
        },
        name: 'params', matches: ['<all_urls>'], all_frames: true, run_at: 'document_start',
    },
    {
        js: {
            files: ['contentscript.js']
        },
        name: 'scripts', matches: ['<all_urls>'], all_frames: true, run_at: 'document_start',
    },
]);

contentscript.js:

try {
    console.log(params.extensionId);
} catch (err) {
    console.error(err);
    alert(`Broken, check devtools > console > error \n\n ${err}`);
}

@panther7
Copy link
Author

Another Example without contentscript.js, all is in code. Broken too...

2020-11-27 12 24 28

{
    name: 'scripts-1',
    js: {
        code: `
            function sendMessage(data) {
                let extensionId = '${location.hostname}';
                return new Promise(resolve => {
                    chrome.runtime.sendMessage(extensionId, data, resolve);
                });
            }

            (async () => {
                if (window.top === window) {
                    try {
                        const response = await sendMessage({ greetings: 'hello' });
                        console.log(response.greetings);
                    } catch (err) {
                        console.error(err);
                        alert(\`Broken, check devtools > console > error \\n\\n $\{err\}\`);
                    }
                }
            })();
        `,
    },
    matches: ['<all_urls>'],
    all_frames: true,
    run_at: 'document_start',
}

@rogerwang

@panther7
Copy link
Author

Wow! Works! Thanks @rogerwang

It was very difficult to come up with more and more examples. :)

@rogerwang
Copy link
Member

@panther7
Copy link
Author

@rogerwang Thanks, when is scheduled release v0.50.2?

@gpetrov
Copy link

gpetrov commented Dec 9, 2020

It is already released

@panther7
Copy link
Author

panther7 commented Apr 8, 2021

I see this problem again, but on a smaller scale, from NWjs 0.52 (maybe 0.51 also).

Please, could you check it? @rogerwang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants