bbc / codext

VS Code's editor shipped as a browser extension.
Apache License 2.0
46 stars 10 forks source link

Tab becomes unresponsive when Codext is enabled #2

Closed danielthepope closed 5 years ago

danielthepope commented 5 years ago

I tried to sign in to OneDrive on Firefox while Codext is enabled. When prompted for my password after putting my email address at https://onedrive.live.com/about/en-gb/signin/, Codext appears to be activated (its icon appears in the address bar), then it gradually maxes out my CPU and RAM. The tab becomes unresponsive and I'm unable to enter my password.

It's difficult to work out what's happening, but if I disable Codext this doesn't happen.

I can't reproduce the issue on Chrome.

PyvesB commented 5 years ago

Hello @danielthepope,

Thanks for the report. However, I just tried this in Firefox 68.0.1 on macOS Mojave and I cannot reproduce the described issue, I managed to log into OneDrive happily. Could you try debugging the extension and tell us more about which bit of code is making the tab unresponsive?

Cheers,

Pyves

danielthepope commented 5 years ago

I added some logging for each time the extension was invoked and for each time Codext loads the editor.

In Firefox 68.0.1 (on Windows at least), the extension is invoked 99 times in the first few seconds, and the editor is loaded 96 times, all of which for other origins e.g. https://spoprod-a.akamaihd.net/files/odsp-next-prod_2019-07-12_20190718.003/aria-bfcb437b.js, which would explain why my laptop goes into a mini-meltdown.

Chrome only seems to invoke the extension 3 times for the same page, and none of them actually load the editor.

I was able to "fix" the issue by setting "all_frames": false in the manifest.json, but I imagine it was set to true for a good reason...

PyvesB commented 5 years ago

I was able to "fix" the issue by setting "all_frames": false in the manifest.json, but I imagine it was set to true for a good reason...

To be honest, I don't believe there is a good reason.

To get us going quickly given the limited time of the hack day, we used one of my personal browser extensions, night-video-tuner, as a starting point. That one does it for a good reason, the video is not always in the top frame so you need to inject scripts everywhere for it to work properly; this is the case for BBC iPlayer for instance.

We left the option in Codext as we thought it wouldn't do any harm and could cover potential edge cases where the "unique pre tag" was not in the top frame. However, I don't know of any, and I suggest we remove the "all_frames": true override in manifest.json as it's causing pain in Firefox. What do you think? Would you like to submit a pull request?