Closed Bug 693261 Opened 13 years ago Closed 13 years ago

registerContentHandler() fails in SeaMonkey's WebContentConverter.js

Categories

(SeaMonkey :: Feed Discovery and Preview, defect)

SeaMonkey 2.5 Branch
defect
Not set
normal

Tracking

(seamonkey2.6 fixed, seamonkey2.7 fixed, seamonkey2.8 fixed)

RESOLVED FIXED
seamonkey2.7
Tracking Status
seamonkey2.6 --- fixed
seamonkey2.7 --- fixed
seamonkey2.8 --- fixed

People

(Reporter: info, Assigned: philip.chee)

References

()

Details

Attachments

(2 files, 2 obsolete files)

Attached file causes Exception in error console (obsolete) —
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).
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.
Version: SeaMonkey 2.2 Branch → Seamonkey 2.5 Branch
We need to port a whole bunch of bugs from Firefox that implement web protocol handlers.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: General → Feed Discovery and Preview
OS: Linux → All
QA Contact: general → feed.handling
Hardware: x86_64 → All
Attachment #565889 - Attachment mime type: text/plain → text/html
Attached file Test Case
Updated Test Case.
Assignee: nobody → philip.chee
Attachment #565889 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Summary: registerContentHandler() fails in SeaMonkey's out-of-date(?) WebContentConverter.js → registerContentHandler() fails in SeaMonkey's WebContentConverter.js
Attached patch Patch v1.0 Fix Typo. (obsolete) — Splinter Review
Simple typo fix browserElement->aContentWindow
Attachment #572205 - Flags: review?(iann_bugzilla)
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
Attachment #572205 - Attachment is obsolete: true
Attachment #572205 - Flags: review?(iann_bugzilla)
Attachment #572257 - Flags: review?(iann_bugzilla)
> (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; 
>    }
(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.
Attachment #572257 - Flags: review?(iann_bugzilla) → review+
I wonder if there is any way of tapping into the tabbrowser.xml getNotificationBox - outside scope of this bug anyway.
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 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.
Attachment #572257 - Flags: approval-comm-beta?
Attachment #572257 - Flags: approval-comm-aurora?
Attachment #572257 - Flags: approval-comm-beta?
Attachment #572257 - Flags: approval-comm-beta+
Attachment #572257 - Flags: approval-comm-aurora?
Attachment #572257 - Flags: approval-comm-aurora+
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
Attachment #572257 - Flags: approval-comm-aurora+ → approval-comm-aurora-
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.7
You need to log in before you can comment on or make changes to this bug.