getstation / electron-chrome-extension

Implementation of Chrome extension APIs for Electron
139 stars 26 forks source link

window.open [APP-200] #38

Closed hugomano closed 5 years ago

hugomano commented 5 years ago

What is this PR

This PR is intended to fix a problem with message forward between windows context. With Mixmax we can start the extension correctly but at the onboarding, when the google modal triggers the extension callback, there is an error withpostMessage that doesn't work because the opener is undefined (ref: https://github.com/electron/electron/issues/16200).

Important: this window-setup is compatible with our current one wich is here

How does it work: how to fix the bug, and how it was implemented

This bunch of code polyfill the window.opener in the opened window since there is an issue on the electron, change the dispatched event type and add a Mixmax specific code block.

We use MessageEvent instead of Event

The small diff is that instead of using Event for forwarding postMessage we use MessageEvent

    event = document.createEvent('Event');
    event.initEvent('message', false, false);
    event.data = message;
    event.origin = sourceOrigin;
    event.source = getOrCreateProxy(ipcRenderer, sourceId);
    window.dispatchEvent(event);

is now

     event = new MessageEvent('message', { data: message, origin: sourceOrigin });
     event.source = getOrCreateProxy(ipcRenderer, sourceId);
     win.dispatchEvent(event);

We do this because jQuery sucks in the event detection on webapp and extension side. Should be backported in Station and Electron (we spent a lot of time to find the root source of the bug) not a prerequisite for now.

We add a small chunk for Mixmax

     if (event.data && event.data.method === 'loginFinished') {
       win.location.reload();
     }

We added the code block because the event propagation is blocked at the iframe level (iframe CORS origin blocked in Electron).

It's a workaround for this Electron issue: https://github.com/electron/electron/issues/9581

The window.open that opens the authentication popup is being called from an iframe, and given preload are not running in iframe, the override of window.open is not present.

The window.open is triggered from a webFrame. The opened window use postMessage that forwards to the content-script correctly. The propagation stops to work at the iframe window context where lives a proxy between Mixmax extension, and Mixmax web app doesn't catch the onMessageEvent. We tried to override the iframe context with webFrame.getFrameForSelector but it is not possible right now since there is no preload for <iframe>.

Test instructions

using Mixmax extension

Before this PR

$ npm run playground:reset
$ npm run start

With this PR

$ npm run playground:reset
$ npm run start
alexstrat commented 5 years ago

there is an error withpostMessage that doesn't work because the opener is undefined

Any idea of the root cause of this? Worth adding it for reference.

alexstrat commented 5 years ago

The whole window-setup file is here to polyfill this Electron issue: electron/electron#9961

~What is the issue and how is the polyfill solving it? electron/electron#9961 seems to be the exact bug the original implementation was trying to solve.~

OK I understood. I thught there was a diff with the original implementation.

alexstrat commented 5 years ago

Should be backported in Station and Electron (we spent a lot of time to find the root source of the bug).

For Electron, worth at least opening an issue.

For Station, what is the plan to not forget about it?

alexstrat commented 5 years ago

The window.open is triggered from a webFrame. The opened window use postMessage that forwards to the content-script correctly. The propagation stops to work at the iframe window context where lives a proxy between Mixmax extension, and Mixmax web app doesn't catch the onMessageEvent. We tried to override the iframe context with webFrame.getFrameForSelector but it is not possible right now since there is no preload for