Electron bump #1419
Electron bump #1419
Conversation
|
Couldn't help myself and debugged a bit more. |
black-puppydog
force-pushed
the
electron-bump
branch
3 times, most recently
from
4c31e7e to
fbdda4e
Compare
| webPreferences: { | ||
| nodeIntegration: true // XXX: Maybe not always necessary (?) | ||
| nodeIntegration: true, // XXX: Maybe not always necessary (?) | ||
| enableRemoteModule: true, |
There was a problem hiding this comment.
The goal is to be able to remove this line again.
| rootPath: path, | ||
| config: config, | ||
| data: opts.data || '', | ||
| title: opts.title || 'Patchwork', |
There was a problem hiding this comment.
If we can pass this somehow directly to the BrowserWindow upon creation, we can save a round-trip of ipc...
There was a problem hiding this comment.
Also (and this is not a new issue, but nevertheless) I am thinking to strip config.keys.private out of this payload, to prevent the renderer process from accessing private key material. Not only should that improve the security in case of e.g. remote code execution CVEs, but also it should remove a potential foot-gun by only having one point in the code being even able to sign new messages, thus potentially preventing bug-induced forks...?
| } | ||
|
|
||
| function dropTab (title, items) { | ||
| function buildDropdownMenuItems() { |
There was a problem hiding this comment.
In hindsight, this could be done in the main process.
But even then, I'd probably build it like this for legibility and keep the buildMenu() around to add the navigate-to event handlers afterwards. So... dunno, feels okay to have the description of the page structure in the renderer process, that's where the rest of it lives too, right?
These were left-overs from a 4 year old commit. :P
Electron introduced native spell-checking in version 8, and this library was subsequently abandoned.
black-puppydog
force-pushed
the
electron-bump
branch
2 times, most recently
from
b76e1f8 to
c3b4607
Compare
To the latest version. Seems to work, mostly. There's some slowness happening in the search suggestions & completions, and of course there's still no context menu. But posting & scuttling & reading still works, so there's that. This still relies on the `remote` module, which is deprecated and considered harmful. Need to work around that in the future, which... makes sense.
First step of getting remote out of the codebase.
Another step towards abolishing `remote`.
We were literally sending over a bunch of javascript code, and using remote to execute it. Now it's done via a handler.
This one was a bit tougher. Basically I'm now creating a *descriptive* structure on the renderer side, and then adding the event handlers on the server side. The event handlers then simply trigger a navigation IPC message.
The old menu was handling more than we needed: it was doing spell-checking, which has been built into electron since v8.0.0, and it was also overly complicated. The new version should be a bit easier to read. Avoiding use of the remote module means that a lot of things have to be handled via explicit IPC (instead of relying on implicit proxy objects from the remote module) and especially for the "Copy Message Text" and "Copy Message Reference" items and such I had to hack a bit. As a result, this *might* still have race conditions that would mean that text doesn't get copied, or an outdated version of it.
black-puppydog
force-pushed
the
electron-bump
branch
from
c3b4607 to
4d9f238
Compare
This is very much WIP, but maybe folks want to test it...?
This is still lacking the context menu and for some reason the completion boxes (in search bar and post textareas) are slow as hell.
Apart from that it seems to work... 🙂