When I close the BrowserWindow in the "hello world" app, the devtools window also closes, but the app doesn't quit. On Mac, that's expected, because the main script leaves the app running on Mac after all windows have closed; but I see this on Linux too (Ubuntu 14.04).

When the <iframe mozbrowser> in the shadow DOM of the <webview> element is remote, it doesn't render its contents. This appears to be a regression from the way we give the BrowserWindow document chrome privileges by assigning it the system principal in nsDocument::reset. We currently work around it by making the iframe not be remote.

That way of giving the BrowserWindow document chrome privileges is probably incorrect, and a fix for that might end up fixing this issue. But I'm filing it here to track it discretely, since it's potentially a separate issue.

We should make the mozbrowser iframe remote because it's more consistent with the behavior of Electron (which creates a renderer process for each webview) and has all the other benefits of e10s wrt. chrome UI responsiveness.

The webview-element branch #60 equates the webview event load-commit, as described in Electron webview docs, with the mozbrowser event mozbrowserlocationchange.

That might be correct, for the top-level frame, anyway, even though WebAPI/BrowserAPI/Common_Subset equates that event with the mozbrowserloadend.

But mozbrowserlocationchange doesn't appear to fire for sub-frame loads, whereas load-commit apparently does. And I haven't yet confirmed that the events are equivalent even for the top-frame load. Also, the implementation doesn't yet set the url and isMainFrame properties of the event object.

So we still need to implement complete support for the load-commit event.

When the page https://github.com/ in a <webview> tries to access the process global, Positron tries to initialize it, even though the Process.webidl binding is marked [ChromeOnly], and the window's docshell is a content docshell. This eventually leads to an error when a renderer module tries to get a reference to the BrowserWindow for the page. We should figure out why this is happening and stop it, as no page loaded in a <webview> should trigger initialization of the process global.

Instead of defining the Cc, Ci, Cr, and Cu globals at the top of every native binding module, we should make the module loader inject them into each module global, just as it injects Components itself, since almost every native binding module uses some of these (or will, once implemented, for the ones that are currently stubs), and using Components directly is too verbose (which is why use of the shortened globals is so prolific).

After loading a web page in the <webview> element in the sample browser app, the page doesn't respond to key events such as arrow key presses to scroll the page. Also, clicking on the page doesn't blur the location bar.

If the page has a text field, however, then clicking in that field will focus the field and blur the location bar. After which the page will start responding to key events. The default URL in the sample browser app, http://github.com/, happens to focus a text field on load, so it employs this workaround (albeit inadvertently).

A bunch of the GuestWebContentsPrototype methods check if (this._webView) before acting, because they're sometimes called before _this.webView has been assigned. We should ensure that it's defined before any of these methods get called.

This is the renderer process half of #19.

The positron/electron/ and positron/node/ directories contain code imported from the https://github.com/electron/electron and https://github.com/nodejs/node/ repositories, respectively. But since the code is imported without a revision/branch identifier, it isn't clear what version was imported, which makes it hard to update.

That's ok right now, when we're just working on getting Positron working like a given version of Electron and Node. But it'll become a problem as soon as we update those files for the first time. So we should come up with a strategy for updating these files that clearly identifies the version of Electron we are currently emulating and the version of Node we're using.

FWIW, the Electron files currently in the repository come from an Electron clone at https://github.com/electron/electron/tree/6041c7edf9fc8ff4d73b0c311200c3953127da3e, while the Node files come from a Node clone at https://github.com/nodejs/node/tree/4bc1cccb228fdfd1a527de0ee1d9cedc6dac87e2.

The webview-element branch #60 works around Mozilla bug 449811 by redeclaring for…of variables within their blocks. Once that bug is fixed, we should remove those workarounds.

https://github.com/mozilla/positron/blob/master/positron/electron/lib/browser/rpc-server.js#L46 checks if a prototype object is strictly equal to Object.prototype, i.e. proto === Object.prototype. But the prototype can come from a different sandbox than the one in which the comparison is taking place, in which case the condition evaluates to false, even if proto is an Object.prototype, perhaps because each sandbox gets its own unique instance of Object, so it isn't the same Object.protoype.

In #26, I work around the problem with a duck-type comparison. But that requires changing the code, which comes from upstream Electron. Ideally, we wouldn't have to modify the Electron code in order to ensure that conditionals like this one behave as it expects.

In this task run, Git complained about the user's identity:

[taskcluster-vcs] 0 run start : (cwd: /repo) git merge origin/master

*** Please tell me who you are.

Run

git config --global user.email "[email protected]"
git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'root@9defe6dad430.(none)')

We should be able to fix this with commands like this in the task configuration:

  git config --global user.email "[email protected]"
  git config --global user.name "Positron Mozilla"

We're now able to open a debugger for the Positron platform by starting with --jsdebugger. This opens a toolbox similar in concept to Firefox's Browser Toolbox that runs from a separate process.

However, the actors give various errors at the moment:

Console says: resource://devtools/server/actors/webconsole.js, line 453: TypeError: argument is not a global object
Inspector says: TypeError: window is null

In general, I think this is related to the shortcuts we took to allow the tools to start very early before a window appeared.

Electron uses a native API to register JavaScript callbacks on destruction of certain objects:

https://github.com/mozilla/positron/search?l=javascript&q=setDestructor&utf8=%E2%9C%93

Electron uses these callbacks to destroy objects in another process that are proxies or callbacks for the objects being destroyed in this process. We'll need to support these callbacks too, or find another way to ensure that the objects in the other process are destroyed, or else we'll leak the objects in the other process.

WebContents.canGoBack/canGoForward are synchronous methods, but the mozbrowser equivalents getCanGoBack and getCanGoForward are async, so our implementations of those methods spin the event loop while waiting for the mozbrowser calls to return. We should figure out a better way to implement those methods.

In the renderer process, we can't yet load all modules in a single sandbox, because attempts to reference Window properties on the target object then throw errors like: TypeError: 'get location' called on an object that does not implement interface Window.

This is described as "TypeError: invalid Array.prototype.sort argument" in the MDN docs on the error. But that's clearly not the issue here. Some code is deciding that the target object doesn't implement Window, even though its prototype is a Window instance.

Whereas that code does think that a sandbox with a Window prototype implements Window. So we work around the problem by loading modules in their own sandboxes in renderer processes. But we should resolve the underlying issue so we can load them in the same sandbox to avoid issues like #45, which we worked around in #26.

(That issue happens to be specific to the main process. But similar issues could crop up in the renderer process.)

At the moment, nothing sets WebViewImpl.attached/elementAttached, although presumably both of those get set at some point in the Electron initialization process for a <webview> element. We should figure out when to set those two properties and set them!

The "hello world" app checks process.platform to determine whether or not to quit the app after all windows have closed (it keeps the app running on Mac; quits otherwise), so we should implement it.

I'm looking for some insight into your app model. I'm assuming it's in your wiki, but I can't seem to access it for some reason.

It should be possible to connect devtools in Firefox to a Positron process in order to debug Positron's own code.

It'd be useful to benchmark the perf of Positron versus Electron, to get a sense of how they differ, and where we're slow. I'm not sure exactly what kind of perf to measure, but some ideas include: web page rendering in a BrowserWindow, web page rendering in a <webview> element, module loading, and &webview> element creation/insertion.

@brendandahl, #91 appears to have regressed GuestWebContentsPrototype.getURL, as _this.webView.browserPluginNode.contentDocument is null. So the URL bar in the sample browser becomes empty after loading a page. Which makes sense, since the document is now in the remote process, so it can't be dereferenced in the local one.

Perhaps the solution here is to maintain the current value of the URL locally, updating it each time it changes via the mozbrowserlocationchange event handler.

@jryans After merging from upstream in #37, Hello World again doesn't quit when I close its window, so upstream appears to have regressed your fix for #29.

The "hello world" app invokes BrowserWindow.webContents.openDevTools() to open a Chromium devtools frame in its main window in Electron. Positron should do the same with Gecko devtools.

WebAPI/BrowserAPI/Common_Subset equates Google's and Microsoft's canGoBack/canGoForward methods with Mozilla's getCanGoBack/getCanGoForward methods. But Electron's equivalents (presumably based on those in Chromium) are synchronous, whereas Mozilla's are asynchronous, returning a promise that resolves to the result.

So our <webview> implementation, which uses an <iframe mozbrowser> internally, emulates them by calling getCanGo* on the mozbrowser and then spinning the event loop until they return. That works, but it's a hack, and it has undesirable side-effects (like hanging the main process). We should figure out a better implementation.

Perhaps a mozbrowser can pro-actively synchronize back/forward status from its child frame to its parent element, so Positron can call synchronous methods on the element or introspect properties that contain its current status. Or perhaps locationchange events can include information on the back/forward status of the mozbrowser, so Positron can keep track of the state itself, as suggested by bug 1198473.

That bug, and bug 1198479, address similar concerns, but they're a bit more general, and they're in FirefoxOS::Runtime, whereas this is probably a Core::DOM issue. So I filed bug 1279635 on it.

Electron opens devtools for a BrowserWindow inside the window, similarly to the way Firefox opens devtools for a tab in a frame at the bottom of the same window. Positron will initially open devtools in a new window, per #16, but eventually it should open devtools in the same window, like Electron.

The webview-element branch #60 works around Mozilla bug 992245 (lack of support for :host pseudo-class) by setting the display property for the webview element via the element's style property. Once that bug is fixed, we should remove that workaround.

Per the WebIDL spec, document.registerElement requires custom element names to contain a dash. But the Electron <webview> element doesn't contain one, because Electron uses the Blink API WebDocument::registerEmbedderCustomElement to register it. We should implement a similar API for Gecko, so Positron can register <webview> and other elements that don't have dashes in them.

@jryans, @ochameau: Sometimes on startup I see this stack dumped to the terminal window, and the devtools window is blank:

A coding exception was thrown and uncaught in a Task.

Full message: TypeError: docShell.chromeEventHandler is null
Full stack: Helpers_onLocationChange@resource://devtools/client/shared/DOMHelpers.jsm:159:7
Toolbox.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:371:7
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:454:5
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:395:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:13
TaskImpl@resource://gre/modules/Task.jsm:280:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
Task_spawn@resource://gre/modules/Task.jsm:168:12
Toolbox.prototype.open@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:357:12
DevTools.prototype.showToolbox@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/devtools.js:435:7
this.gDevTools[name]@resource://devtools/client/framework/gDevTools.jsm:107:12
toolboxOpened<@chrome://devtools/content/framework/toolbox-init.js:87:14
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:714:7
TabTarget.prototype.makeRemote/onConsoleAttached@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:410:7
DebuggerClient.prototype.attachConsole/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:552:7
resolve@resource://devtools/shared/deprecated-sync-thenables.js:40:40
then@resource://devtools/shared/deprecated-sync-thenables.js:20:43
resolve@resource://devtools/shared/deprecated-sync-thenables.js:72:11
listenerJson@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:740:9
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
Request.prototype.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1234:29
DebuggerClient.prototype.onPacket/emitReply@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1018:29
DevTools RDP*DebuggerClient.prototype.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:724:5
DebuggerClient.prototype.attachConsole@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:542:12
TabTarget.prototype.makeRemote/attachConsole@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:414:7
TabTarget.prototype.makeRemote/attachTab/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:400:9
DebuggerClient.prototype.attachTab/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:467:7
resolve@resource://devtools/shared/deprecated-sync-thenables.js:40:40
then@resource://devtools/shared/deprecated-sync-thenables.js:20:43
resolve@resource://devtools/shared/deprecated-sync-thenables.js:72:11
listenerJson@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:740:9
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
Request.prototype.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1234:29
DebuggerClient.prototype.onPacket/emitReply@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1018:29
DevTools RDP*DebuggerClient.prototype.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:724:5
DebuggerClient.prototype.attachTab@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:461:12
TabTarget.prototype.makeRemote/attachTab@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:392:7
TabTarget.prototype.makeRemote@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:432:7
exports.TargetFactory.forRemoteTab@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:59:23
exports.targetFromURL<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target-from-url.js:118:10
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
resolve@resource://devtools/shared/deprecated-sync-thenables.js:40:40
then@resource://devtools/shared/deprecated-sync-thenables.js:20:43
resolve@resource://devtools/shared/deprecated-sync-thenables.js:72:11
listenerJson@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:740:9
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
Request.prototype.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1234:29
DebuggerClient.prototype.onPacket/emitReply@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1018:29
DevTools RDP*DebuggerClient.prototype.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:724:5
RootClient.prototype.getWindow@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1674:12
exports.targetFromURL<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target-from-url.js:102:28
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
resolve@resource://devtools/shared/deprecated-sync-thenables.js:40:40
then@resource://devtools/shared/deprecated-sync-thenables.js:20:43
resolve@resource://devtools/shared/deprecated-sync-thenables.js:72:11
DebuggerClient.prototype.connect/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:360:7
eventSource/aProto.addOneTimeListener/l@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:68:7
eventSource/aProto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:131:9
exports.DebuggerClient/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:227:5
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
Request.prototype.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1234:29
DebuggerClient.prototype.onPacket/emitReply@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1018:29
DebuggerClient.prototype.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1023:9
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14

We've stubbed a variety of modules and the functions they expose, but none of the stubs warn that they're stubbly, which is dangerous, because it makes them easy to overlook when troubleshooting an issue that may be caused by an incomplete or missing implementation.

So we should make those stubs warn (once per session) when the stubbed modules are evaluated and the stubbed functions are invoked.

The "hello world" app observes the window-all-closed event on the app module, so Positron should emit that event when all BrowserWindows are closed.

@jryans If Positron is paused in the remote debugger, and I quit it, then I see two of these error messages in the terminal:

---

A coding exception was thrown in a Promise resolution callback.
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Full message: TypeError: this._extraPools is null
Full stack: DSC_removeActorPool@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1423:9
ThreadActor<._resumed@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1583:5
ThreadActor<.onResume/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1031:20
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:454:5
ThreadActor<.onResume@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1023:12
ThreadActor<.disconnect@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:577:7
AP_remove@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:263:7
AP_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:225:7
DSC_removeActorPool/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1427:33
DSC_removeActorPool@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1427:9
_popContext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1465:5
_detach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1502:5
ChromeActor.prototype._detach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/chrome.js:156:3
exit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1125:9
disconnect@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webbrowser.js:1108:5
AP_remove@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:263:7
AP_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/common.js:225:7
DSC_onClosed/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1743:41
DSC_onClosed@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1743:5
DebuggerTransport.prototype.close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:212:9
DebuggerServerConnection.prototype.close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1387:5
DS_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:215:7
BrowserToolboxProcess.prototype.close@resource://devtools/client/framework/ToolboxProcess.jsm:252:7
EventLoop.prototype.enter@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:349:5
ThreadActor<._pushThreadPause@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:542:5
ThreadActor<._pauseAndRespond@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:752:7
ThreadActor<.onDebuggerStatement@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/script.js:1828:9
receiveMessage@resource:///modules/ModuleLoader.jsm -> resource:///modules/gecko/atom_renderer_ipc.js:32:1

I don't see them if I quit the remote devtools process first, nor do I see them if the remote devtools are open, but the Positron process isn't paused in the remote debugger.

Per #54 (comment), opening multiple windows will cause IPC to fail for the second and subsequent, because BrowserWindow.prototype.receiveMessage returns an array of values from all BrowserWindow instances, while atom_renderer_ipc.sendSync returns only the first value from that array. We're only getting away with it because we've only been testing with apps that have one window.

To fix the problem, we might make BrowserWindow.prototype.receiveMessage be a static method that receives all IPC messages and routes them to the correct BrowserWindow instance.

Per #60 (comment), loading a URL whose CSP forbids framing (like https://github.com/) into a <webview> fails with an alert dialog that says, "This page has a content security policy that prevents it from being loaded in this way."

The dialog is triggered by nsDocument::InitCSP, presumably because that function thinks it's being loaded in a frame, since there's an mDocumentContainer docshell for the mozbrowser.

https://dxr.mozilla.org/mozilla-central/rev/4d63dde701b47b8661ab7990f197b6b60e543839/dom/base/nsDocument.cpp#2948

Perhaps we just need to check nsDocShell::GetIsMozBrowserOrApp before deciding whether or not to enforce the frame-ancestor policy, although it returns false in this case, as mFrameType is FRAME_TYPE_REGULAR for the docshell, which seems wrong. So perhaps it's being set incorrectly somewhere.

It could also be an issue that nsCSPContext::PermitsAncestry generates ancestorsArray by walking the docshell tree until it reaches a docshell whose document URI scheme is chrome, which won't be the case here, as the URL of the document that contains the mozbrowser will be a file: URL like file:///path/to/positron/test/hello-world/index.html.

Currently, the Positron repository is a fork of gecko-dev (i.e. mozilla-central), with Positron-specific code living in a positron/ subdirectory, and the Gecko code (along with the code for all other core Mozilla apps) living in the top-level directory.

This makes the codebase more complicated to grok than the Electron and Node codebases, both of which (like many other projects with dependencies) put their own code in the top-level directory and their dependencies in a subdirectory such as deps/.

It may be the case that we'll want to merge Positron code into mozilla-central in the future, making it a core Mozilla app like b2g/ and browser/. But it's also possible that we'll maintain it as a separate codebase. And it'll be easier to hack on, especially for contributors who aren't familiar with mozilla-central, if we isolate Gecko code into a separate directory.

So we should invert the relationship between the Positron application and its Gecko dependency, putting the Positron code into the top-level directory and the gecko-dev code into a subdirectory.

The "hello world" app observes the closed event on its mainWindow, so BrowserWindow should emit that event when it's closed.

The "hello world" app calls app.quit to quit the app when all windows have closed (unless Mac), so we should implement it.

When you press the Go button in the webview/browser sample app, the app sets the src property of the <webview> element, which triggers a call to the attributeChangedCallback on that element in Electron, but doesn't trigger a call to that function in Positron. We should figure out if this is a bug in Positron or a difference in the behavior of Gecko vis-a-vis Blink when modifying properties of a custom element.

The <webview> element in Electron provides an unprivileged frame for loading untrusted web content along with an API for interacting with it, similar to <iframe mozbrowser> in Gecko.

It's implemented in Electron by web-view.js, which calls the native API WebFrame::RegisterEmbedderCustomElement to register the <webview> and <browserplugin> custom elements (as subclasses of HTMLObjectElement) and then exports a WebView() constructor.

The WebView() constructor then creates a <webview> instance, creates a <browserplugin> instance, creates a shadow root on the <webview>, and appends the <browserplugin> to the shadow root.

For Positron, we should be able to enable Web Components, use the standard document.registerElement to register a custom <webview> element whose shadow root contains an <iframe mozbrowser>, and then translate <webview> APIs to <iframe mozbrowser> APIs.

We'll probably have to modify web-view.js in this case (although I've been avoiding modifications to the Electron scripts in positron/electron/lib/ generally, since it'll be easier to track upstream changes the less we modify that code).

Also note browserhtml/browserhtml#639, which is about designing a <webview> implementation for Servo/browser.html. Of note, that issue references this comparison by Kan-Ru of <webview> and <iframe mozbrowser>.

On first paint, the <webview> overlaps the navigation bar in the sample browser app:

Resizing the window fixes the problem, and @brendandahl doesn't see it on his machine, where the window opens in a second screen (which may trigger a resize or otherwise cause a repaint).

It should paint correctly the first time.

Giving a BrowserWindow the system principal, which I did on the webview-branch (since merged to master), causes the BrowserWindow document body to appear empty in the Inspector devtool's tree view, when I open the Inspector for a given BrowserWindow. And opening the Inspector triggers these errors in the console:

Error writing response to: childrenprotocol.js:1058
TypeError: 'get contentDocument' called on an object that does not implement interface HTMLObjectElement.
Stack trace:
get numChildren@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:352:1
exports.NodeActor<.form@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:262:7
types.addActorType/type<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:294:44
types.addArrayType/<.write/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:199:40
types.addArrayType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:199:24
types.addDictType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:233:23
RetVal<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:529:12
Response<.write/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:682:16
Response<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:680:23
generateRequestHandlers/</handler/sendReturn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1056:24
Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:454:5
generateRequestHandlers/</handler/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1076:18
Actor<._queueResponse@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:920:20
generateRequestHandlers/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1075:9
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1648:15
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:568:13
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
protocol.js:907
Protocol error (unknownError): 'get contentDocument' called on an object that does not implement interface HTMLObjectElement.Promise-backend.js:940

@jryans Perhaps Inspector is not designed to support chrome HTML documents? Or maybe the way we give the system principal to the BrowserWindow document, which isn't at a chrome: URL, is the culprit?

The webview-element branch #60 gives loads in top-level docshells the system principal so Positron can give chrome privileges to application documents that it loads into chrome windows from file: URLs. That works, but it's suboptimal, because it doesn't distinguish between application documents and other documents that load into top-level docshells (like the hidden window). There may also be some security implications.

So we should figure out a better way to give chrome privileges to these documents (and upstream it to Gecko).

atom_renderer_ipc inserts a null value into the list of values it passes to ipcRenderer event listeners when it receives an ipc-message. The value represents an event parameter.

It's clearly wrong to set event to the null value, although it works for the single message that atom_renderer_ipc currently receives, since the handler for that message ignores the event parameter.

But we should figure out what it's actually supposed to be.

In 64e55b9 (part of #60), WebViewImpl twiddles the display property of the <webview> node directly, since Gecko doesn't yet support the :host pseudo-class, per bug 992245. Once Gecko supports that pseudo-class, however, we should remove the hack.

We should make it possible to publish Positron to NPM so testers can start testing it with their own Electron apps. Right now it won't run most of those apps, so the NPM package won't be very useful. But it'll become more useful over time. And this work can be done in advance, so we have packages available the moment Positron becomes useful enough to advertise to a larger audience of testers.

There's already some code in the tree to make the the dist/ directory into an Node module, specifically the positron/package.json and positron/cli.js files (and the code in positron/Makefile.in to copy them to the appropriate place).

In atom_browser_web_contents.js we need to emit an event with more information so NavigationController tell if the location change was inPage and if it is suppose to replaceEntry.

The webview-element branch #60 sets the mozbrowser element's internalinstanceid attribute in a timeout inside its createdCallback, because setting it immediately in that callback causes the initial URL not to load. The timeout works, but we should figure out where/when to actually set that attribute.

When Electron starts a process, it creates a Node environment with an init.js script path:

https://github.com/electron/electron/blob/d62d8bbf0aea08e1ab32edf741335c37f92cc58a/atom/common/node_bindings.cc#L163-L166

The init.js script then initializes the environment before running the Electron app.

Even though we don't yet have a real Node implementation, we should be able to evaluate the init.js scripts, stubbing out (or minimally implementing) the Node API calls they invoke, in order to configure the Positron environment similarly to Electron.

This is more of a meta-issue, as the work entrains a variety of separable changes.

At the very least, the work to support the browser (a.k.a. main) process's init.js script (https://github.com/mozilla/positron/blob/master/positron/electron/lib/browser/init.js) can be separated from the work to support renderer processes' init.js script (https://github.com/mozilla/positron/blob/master/positron/electron/lib/renderer/init.js). Pieces like #18 can also be spun off and implemented independently.

We should integrate changes continuously, using Travis CI to build and test pull requests and merges. The Electron unit tests don't work yet, but the "hello world" app does, so the "test" portion can simply run that app and check the outcome. We may need to hack the app to auto-quit and report results.