nsIWebContentHandlerRegistrar::registerProtocolHandler doesn't work in e10s

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: markh, Assigned: mrbkap)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox42 fixed)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

5 years ago
either that, or just the test is broken, but when run in e10s mode:

2:27.50 TEST-START | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js
 2:27.52 *** registerProtocolHandler(testprotocol,https://example.com/foobar?uri=%s,Test Protocol)
 2:27.52 ************************************************************
 2:27.52 * Call to xpconnect wrapped JSObject produced this error:  *
 2:27.52 [Exception... "'[JavaScript Error: "Permission denied for <http://example.com> to create wrapper for object of class UnnamedClass" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 25}]' when calling method: [nsIWebContentHandlerRegistrar::registerProtocolHandler]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: http://example.com/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.html :: <TOP_LEVEL> :: line 12"  data: yes]
 2:27.52 ************************************************************
 2:27.52 JavaScript error: http://example.com/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.html, line 12: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: '[JavaScript Error: "Permission denied for <http://example.com> to create wrapper for object of class UnnamedClass" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 25}]' when calling method: [nsIWebContentHandlerRegistrar::registerProtocolHandler]
 2:27.53 TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js | Console message: [JavaScript Error: "Permission denied for <http://example.com> to create wrapper for object of class UnnamedClass" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 25}]
 2:27.53 TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js | Console message: [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: '[JavaScript Error: "Permission denied for <http://example.com> to create wrapper for object of class UnnamedClass" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 25}]' when calling method: [nsIWebContentHandlerRegistrar::registerProtocolHandler]" {file: "http://example.com/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.html" line: 12}]
 2:30.66 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js | 100
 2:30.66 Stack trace:
 2:30.66     JS frame :: chrome://mochitests/content/browser/browser/base/content/test/general/head.js :: <TOP_LEVEL> :: line 67
 2:30.66     native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
 2:30.66 
 2:30.66 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js | Notification box should be displayed
 2:30.66 Stack trace:
 2:30.66     JS frame :: chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js :: test/< :: line 21
 2:30.66     JS frame :: chrome://mochitests/content/browser/browser/base/content/test/general/head.js :: <TOP_LEVEL> :: line 82
 2:30.66     JS frame :: chrome://mochitests/content/browser/browser/base/content/test/general/head.js :: <TOP_LEVEL> :: line 68
 2:30.66     native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
 2:30.66 
 2:30.66 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js | uncaught exception - TypeError: notification is null at chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js:22
 2:30.66 Stack trace:
 2:30.66     JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1166
 2:30.66     native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
 2:30.66 
 2:30.66 JavaScript error: chrome://mochitests/content/browser/browser/base/content/test/general/browser_registerProtocolHandler_notification.js, line 22: notification is null
tracking-e10s: --- → +
Blocks: 984139

Updated

4 years ago
Blocks: 1080801
I get the same error trying to use it in the web console, so I think it's legit broken:

navigator.registerProtocolHandler("mailto","https://www.fastmail.com/action/compose/?mailto=%s","FastMail");

[Exception... "[JavaScript Error: "aWindow.QueryInterface is not a function" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 49}]'[JavaScript Error: "aWindow.QueryInterface is not a function" {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 49}]' when calling method: [nsIWebContentHandlerRegistrar::registerProtocolHandler]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: yes]

Comment 2

4 years ago
I have a custom protocol handler (for magnet: scheme) in an addon and it apparently stops working with e10s, although I see no errors in the console.  Works fine in nightly with autostart.1 pref set to false, but just stops handling magnet links when e10s is enabled.

Comment 3

4 years ago
Would like to add FireBible to the list of affected extensions. The sword:// & bible:// protocols that FireBible adds no longer work with e10s enabled, pretty much breaking the extension entirely. http://thegoan.com/firebible

Updated

4 years ago
Blocks: 516752

Updated

4 years ago
Depends on: 1176646
(Assignee)

Updated

3 years ago
Assignee: nobody → mrbkap
(Assignee)

Updated

3 years ago
Blocks: 1175056
(Assignee)

Comment 4

3 years ago
Created attachment 8639627 [details] [diff] [review]
Use the Services jsm to simplify a few lines.

There were a few places that could be simplified by using Services.
(Assignee)

Comment 5

3 years ago
Created attachment 8639628 [details] [diff] [review]
wip for registerProtocolHandler

This should make registerProtocolHandler work, I still need to test it in the e10s case. Unfortunately it appears that test_registerHandler.html requires registerContentHandler to work as well. I have a plan for making that work, dealing with pref branch observers that I'll base on top of this patch.

Comment 7

3 years ago
Does this bug, or will this fix, also cover registerProtocolHandler with nsIProtocolHandler?
(Assignee)

Comment 8

3 years ago
Ian, I don't think this bug will affect that. What API, exactly are you talking about?

Comment 9

3 years ago
I think you're right.  I've finally got a custom scheme working in my addon using the component registrar in a frame script.  It doesn't work from a javascript component any more.  It wasn't working in a frame script, now it is.  I don't know what I was doing wrong.
(Assignee)

Comment 10

3 years ago
Created attachment 8641354 [details] [diff] [review]
Small patch to the test

This test passes the parameters to is() backwards, making it confusing to debug. I corrected that. While I was here, I fixed a todo() that wasn't written correctly either, that became a todo_is, like the rest of them.

Dao, you're the last person to review anything in this file, please let me know if there's a better reviewer for these patches.
Attachment #8639628 - Attachment is obsolete: true
(Assignee)

Comment 11

3 years ago
Created attachment 8641355 [details] [diff] [review]
Fix registerProtocolHandler

I'm playing a little fast and loose with the URI stuff, but I think it's correct.
(Assignee)

Comment 12

3 years ago
Created attachment 8641356 [details] [diff] [review]
Fix registerContentHandler
(Assignee)

Comment 13

3 years ago
Created attachment 8641357 [details] [diff] [review]
Fix an odd bug

According to the comment I'm removing, this isn't supposed to throw, but the function returns undefined in this case and nobody checks for it, so we'd throw anyway. Furthermore, according to the current spec[1], we are supposed to throw.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#dom-navigator-registerprotocolhandler
(Assignee)

Comment 14

3 years ago
Created attachment 8641358 [details] [diff] [review]
Enable this test on e10s.

With these patches, this test now passes.
(Assignee)

Updated

3 years ago
Attachment #8639627 - Flags: review?(dao)
(Assignee)

Updated

3 years ago
Attachment #8641354 - Flags: review?(dao)
(Assignee)

Updated

3 years ago
Attachment #8641355 - Flags: review?(dao)
(Assignee)

Updated

3 years ago
Attachment #8641356 - Flags: review?(dao)
(Assignee)

Updated

3 years ago
Attachment #8641357 - Flags: review?(dao)
(Assignee)

Updated

3 years ago
Attachment #8641358 - Flags: review?(dao)
(Assignee)

Comment 15

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=76161d332813

With these patches, registerContentHandler should work, but I still need to implement nsIWebContentConverterService for the feed page to work properly (the current implementation will try to set prefs from the content process. It should be possible to do this with relatively little work and some refactoring (plus some pref observers). Should I do that in this bug?
Attachment #8639627 - Flags: review?(dao) → review+
Attachment #8641354 - Flags: review?(dao) → review+
Comment on attachment 8641355 [details] [diff] [review]
Fix registerProtocolHandler

>+const Utils = {
>+  _makeURI(aURL, aOriginCharset, aBaseURI) {
>+    var ioService = Components.classes["@mozilla.org/network/io-service;1"]
>+                              .getService(Components.interfaces.nsIIOService);
>+    return ioService.newURI(aURL, aOriginCharset, aBaseURI);

You can use Services.io here.

You can also get rid of the underscore prefixes for these method names since Utils isn't exported from this script.
Attachment #8641355 - Flags: review?(dao) → review+
Comment on attachment 8641356 [details] [diff] [review]
Fix registerContentHandler

>     if (aWindowOrBrowser) {
>-      var uri = Utils._checkAndGetURI(aURIString, aWindowOrBrowser);
>+      var haveWindow = (aWindowOrBrowser instanceof Ci.nsIDOMWindow);
>+      var uri;
>+      var notificationBox;
>+      if (haveWindow) {
>+        uri = Utils._checkAndGetURI(aURIString, aWindowOrBrowser);
>+
>+        var browserWindow = this._getBrowserWindowForContentWindow(aWindowOrBrowser);
>+        var browserElement = this._getBrowserForContentWindow(browserWindow, aWindowOrBrowser);
>+        notificationBox = browserWindow.gBrowser.getNotificationBox(browserElement);

You could use browserElement.getTabBrowser().getNotificationBox(browserElement) here like you did elsewhere.
Attachment #8641356 - Flags: review?(dao) → review+
Attachment #8641357 - Flags: review?(dao) → review+
Attachment #8641358 - Flags: review?(dao) → review+
(Assignee)

Comment 18

3 years ago
Created attachment 8641881 [details] [diff] [review]
With comments addressed
Attachment #8639627 - Attachment is obsolete: true
Attachment #8641354 - Attachment is obsolete: true
Attachment #8641355 - Attachment is obsolete: true
Attachment #8641356 - Attachment is obsolete: true
Attachment #8641357 - Attachment is obsolete: true
Attachment #8641358 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/9d754f7e1cb9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42

Comment 21

3 years ago
Using the nightly build: 42.0a1 (2015-08-04) - my protocol handler is still defunct when e10s is enabled. Is this issue supposed to be fixed in the above build, or have I misunderstood the extent of the fix?

I see a "Firefox doesn't know how to open this address, because one of the following protocols (bible) isn't associated with any program or is not allowed in this context." error. Can't see any protocol related errors in the log though.

When e10s is disabled, the protocol handler works just fine.

Extension can be installed from: http://thegoan.com/firebible for testing - the extension signing check will need to be disabled.

Comment 22

3 years ago
A quick look at your code suggests that FireBible is not using a web content protocol handler.  It is using a chrome custom protocol handler (nsIProtocolHandler).  This is the same confusion I had.  This bug is not relevant. Look at the latest beta versions of my addon for code that works (frame.js is the frame script, frame.jsm contains the handler):
https://addons.mozilla.org/en-us/firefox/addon/torrent-status-tool/

To the best of my knowledge, the only way to use these with e10s enabled is to manually register your protocol handler in a frame script.  I could not find as way to do this using the old chrome.manifest registration.  Protocol handlers registered in a component exist but are never called to handle content.

Updated

3 years ago
No longer depends on: 1176646

Comment 23

3 years ago
Ian, appreciate the explanation, thank you. It does look like the the nsIProtocolHandler issue is being addressed in bug #1176646 though, so I can wait on a fix there before testing further; hopefully changes to the extension will not be required.

Updated

3 years ago
Depends on: 1194585
No longer blocks: 1097530

Updated

3 years ago
No longer blocks: 1095484
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1175056
You need to log in before you can comment on or make changes to this bug.