693261 - registerContentHandler() fails in SeaMonkey's WebContentConverter.js

Status: RESOLVED FIXED

(SeaMonkey :: Feed Discovery and Preview, defect)

Description

[skierpage]

Attachment: causes Exception in error console

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111006 Firefox/8.0 SeaMonkey/2.5
Build ID: 20111006141859

Steps to reproduce:

I tried the Example in https://developer.mozilla.org/en/DOM/window.navigator.registerContentHandler in SeaMonkey, but used a relative URI to avoid getting 'Permission denied to add http://www.example.tld/?foo=%s as a content or protocol handler.'  I.e. just the three lines
  <script>
navigator.registerContentHandler("application/vnd.mozilla.maybe.feed",
                                 "local.html?foo=%s",
                                 "My Feed Reader");
  </script>
which I'll attach.


Actual results:

Error console has the error:

Error: uncaught exception: [Exception... "'[JavaScript Error: "browserElement is not defined" {file: "jar:file:///home/skierpage/programs/seamonkey_2.5b2/omni.jar!/components/WebContentConverter.js" line: 511}]' when calling method: [nsIWebContentHandlerRegistrar::registerContentHandler]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///home/skierpage/Documents/work/ODF_in_Firefox/my_register_bug.html :: <TOP_LEVEL> :: line 4"  data: yes]



Expected results:

I think this should bring up a notification UI.  If I view the WebContentConverter.js from SeaMonkey's omni.jar, it is indeed missing any setting of a browserElement variable before it invokes getNotificationBox(browserElement).

The same jar: reference in Firefox nightly brings up a different version of WebContentConvert.js that does set var browserElement first.   It seems SeaMonkey's WebContentConvert.js file comes from suite/feeds/src/ in the comm-central tree, http://hg.mozilla.org/comm-central/file/tip/suite/feeds/src/WebContentConverter.js#l510 .  The comm-central tree has another copy of WebContentConvert.js that matches Firefox, http://hg.mozilla.org/mozilla-central/file/tip/browser/components/feeds/src/WebContentConverter.js#l490

(FWIW, the same registerContentHandler invocation still fails a few lines later in Firefox nightly, I'll file a separate bug.)

This seems low-priority until registerContentHandler() is enhanced so people can do Boot2Gecko-ish things like pass PDF and ODF URLs to an HTML page that renders them (bug 391286).

Comment 1

[skierpage]

I uploaded the registerContentHandler to a URL.  http://www.skierpage.com/moz_bugs/my_register_bug2.html (with a fully-qualified URI parameter on the same host) works in Firefox nightly but gives this uncaught exception in SeaMonkey 2.5b2.

Comment 2

[Philip Chee]

We need to port a whole bunch of bugs from Firefox that implement web protocol handlers.

Comment 3

[Philip Chee]

Attachment: Test Case

Updated Test Case.

Comment 4

[Philip Chee]

Attachment: Patch v1.0 Fix Typo.

Simple typo fix browserElement->aContentWindow

Comment 5

[Philip Chee]

Attachment: Patch v1.1 with added tests.

Changes since the last patch:
1. Added test_registerHandler.html copied verbatim from Firefox.

$ TEST_PATH=suite/browser/test/test_registerHandler.html  make -C ../objdir-sm/ mochitest-plain

TEST-PASS | unknown test url | navigator.registerProtocolHandler should be defined
TEST-PASS | unknown test url | navigator.registerContentHandler should be defined
TEST-PASS | unknown test url | registering a foo protocol handler should work - true should equal true
TEST-PASS | unknown test url | registering a foo content handler should work - true should equal true
TEST-PASS | unknown test url | a protocol handler uri should contain %s - false should equal false
TEST-PASS | unknown test url | a content handler uri should contain %s - false should equal false
TEST-PASS | unknown test url | a protocol handler uri should be valid - false should equal false
TEST-PASS | unknown test url | a content handler uri should be valid - false should equal false
TEST-PASS | unknown test url | registering a foo protocol handler with a different host should not work - false should equal false
TEST-PASS | unknown test url | registering a foo content handler with a different host should not work - false should equal false
TEST-PASS | unknown test url | registering a foo protocol handler with https scheme should work - true should equal true
TEST-PASS | unknown test url | registering a foo content handler with https scheme should work - true should equal true
TEST-PASS | unknown test url | registering a foo protocol handler with ftp scheme should not work - false should equal false
TEST-PASS | unknown test url | registering a foo content handler with ftp scheme should not work - false should equal false
TEST-PASS | unknown test url | registering a foo protocol handler with chrome scheme should not work - false should equal false
TEST-PASS | unknown test url | registering a foo content handler with chrome scheme should not work- false should equal false
TEST-PASS | unknown test url | registering a foo protocol handler with foo scheme should not work - false should equal false
TEST-PASS | unknown test url | registering a foo content handler with foo scheme should not work - false should equal false
TEST-PASS | unknown test url | registering a chrome protocol handler should not work - false should equal false
TEST-PASS | unknown test url | registering a vbscript protocol handler should not work - false should equal false
TEST-PASS | unknown test url | registering a javascript protocol handler should not work - false should equal false
TEST-PASS | unknown test url | registering a moz-icon protocol handler should not work - false should equal false
TEST-PASS | unknown test url | registering rss content handlers should work - true should equal true
TEST-PASS | unknown test url | registering atom content handlers should work - true should equal true
TEST-KNOWN-FAIL | unknown test url | true - registering html content handlers should not work
Document http://mochi.test:8888/tests/suite/browser/test/test_registerHandler.html?autorun=1&closeWhenDone=1&logFile=c%3A%5Ct1%5Chg%5Cobjdir-sm%5Cmozilla%5Cmochitest-plain.log&fileLevel=INFO&consoleLevel=INFO loaded successfully

Comment 6

[Philip Chee]

> (FWIW, the same registerContentHandler invocation still fails a few lines later in
> Firefox nightly, I'll file a separate bug.)

http://hg.mozilla.org/comm-central/annotate/9da1e377dc9d/suite/feeds/src/WebContentConverter.js#l349
http://hg.mozilla.org/mozilla-central/annotate/c6b2050b04ef/browser/components/feeds/src/WebContentConverter.js#l344

>  function WCCR_checkAndGetURI(aURIString, aContentWindow)
>  {
>    try {
>      var uri = this._makeURI(aURIString);
I think we should at least pass the base url to _makeURI as well

>    } catch (ex) {
>      // not supposed to throw according to spec
ORLY?

>      return; 
>    }

Comment 7

[Ian Neal]

(In reply to Philip Chee from comment #6)
> > (FWIW, the same registerContentHandler invocation still fails a few lines later in
> > Firefox nightly, I'll file a separate bug.)
> 
> http://hg.mozilla.org/comm-central/annotate/9da1e377dc9d/suite/feeds/src/
> WebContentConverter.js#l349
> http://hg.mozilla.org/mozilla-central/annotate/c6b2050b04ef/browser/
> components/feeds/src/WebContentConverter.js#l344
> 
> >  function WCCR_checkAndGetURI(aURIString, aContentWindow)
> >  {
> >    try {
> >      var uri = this._makeURI(aURIString);
> I think we should at least pass the base url to _makeURI as well
If we can then, yes, what about the charset too? Both would be outside of scope of this bug.
Would have to log a bug against Firefox too.

Comment 8

[Ian Neal]

I wonder if there is any way of tapping into the tabbrowser.xml getNotificationBox - outside scope of this bug anyway.

Comment 9

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/ed660e715f22

>> I think we should at least pass the base url to _makeURI as well
> If we can then, yes, what about the charset too? Both would be outside of scope of this
> bug.
> Would have to log a bug against Firefox too.
To clarify. The first test case failed because it used a relative URL for the web content handler. I anticipate that in the wild the urls would be fully qualified absolute URLS.

Comment 10

[Philip Chee]

Comment on attachment 572257 [details] [diff] [review]
Patch v1.1 with added tests.

Not a regression technically. Just never worked but nobody bothered to report this problem before.

Comment 11

[Philip Chee]

Comment on attachment 572257 [details] [diff] [review]
Patch v1.1 with added tests.

comm-aurora:
Already uplifted from comm-central.

Pushed to comm-beta:
http://hg.mozilla.org/releases/comm-beta/rev/6e6c1c75d7b0