pixiebrix / pixiebrix-extension

PixieBrix browser extension
https://www.pixiebrix.com
GNU Affero General Public License v3.0
83 stars 22 forks source link

Content script doesn't load in srcdoc iframes causes message retries #6206

Closed twschiller closed 1 year ago

twschiller commented 1 year ago

Context

Steps to Reproduce

Screenshots

There are multiple "received in content script messages" after the content script responds

image

Discussion

There might be extra copies of the content script in top-level frame. (But only one of them has any logs in the console for the context). I think these were injected by the Page Editor

image

It looks like it's the srcdoc frame, what's interesting is that the brick is not actually being run there though (there's no "Hello" message)

image

fregante commented 1 year ago

I think these were injected by the Page Editor

I don’t think that’s possible anymore, right? The injection is entirely handled by the browser now. There’s no chance that scrips are injected twice (unless the extension is reloaded). The duplicates you see are probably only due to the dev reload.

What does it look like in the background page? My guess is that each of those message has a sender in the background page

fregante commented 1 year ago

The messages are multiple retries of the same message from the background script, this can be also seen in the background:

Screenshot 3

I'm having a hard time debugging this issue because the developer tools appear to ignore the breakpoints in that iframe 🤔

My best guess at the moment is that the application is triggering a silent fatal error and the message doesn't even get to respond with that error. My second guess is that the error happens earlier than the method registration, so the code reaches "No handler registered" and moves forward as a "retriable scenario"

fregante commented 1 year ago

As you mentioned before, all the http checks in the app should at least be updated to include about:, for example:

https://github.com/pixiebrix/pixiebrix-extension/blob/294337ff7761eb4561a610b0df33c0741d267410/src/permissions/permissionsUtils.ts#L184C1-L186

fregante commented 1 year ago

Indeed no methods are being registered in that context:

Screenshot 7

So it causes this error:

Screenshot 6
fregante commented 1 year ago

contentScriptCore: init is not in the console, probably import("contentScriptCore") is failing entirely

https://github.com/pixiebrix/pixiebrix-extension/blob/294337ff7761eb4561a610b0df33c0741d267410/src/contentScript/contentScriptCore.ts#L51C1-L54

Is the @cfworker/json-schema error related? new URL('about:srcdoc').origin === 'null' so this is probably breaking a lot of expectations everywhere. I can work on this issue until it's resolved if you confirm.

https://github.com/pixiebrix/pixiebrix-extension/assets/1402241/b9b7f451-6ce3-4aaa-8733-f77e3f6e1365

twschiller commented 1 year ago

Since it's a module-level import, you're right it'd be preventing the contentScript entry point from loading

Here's the code: https://github.com/cfworker/cfworker/blob/main/packages/json-schema/src/dereference.ts#L68

Would be great if you could create an issue/PR in the cfworker repo. We will likely have to vendor that file in the meantime and swap it out via webpack alias

fregante commented 1 year ago

I just noticed that this is still open and assigned to me. If it's still an issue even after that PR I can keep looking into it

twschiller commented 1 year ago

Thank you for checking, I think we just forgot to close it. Closing now