Closed Bug 995802 Opened 7 years ago Closed 7 years ago

registerProtocolHandler("mailto") terminates js with error and breaks Yahoo Mail [don't throw an exception in nsIWebContentHandlerRegistrar if protocol is to be handled internally]

Categories

(SeaMonkey :: Feed Discovery and Preview, defect)

SeaMonkey 2.22 Branch
defect
Not set
major

Tracking

(seamonkey2.26 fixed, seamonkey2.27 fixed, seamonkey2.28 fixed)

RESOLVED FIXED
seamonkey2.28
Tracking Status
seamonkey2.26 --- fixed
seamonkey2.27 --- fixed
seamonkey2.28 --- fixed

People

(Reporter: michal-ok, Assigned: neil)

References

()

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101 Firefox/28.0 SeaMonkey/2.25 (Beta/Release)
Build ID: 20140318183706

Steps to reproduce:

After logging in to the new interface of web mail at yahoo.com the mail app is almost unusable - can't read nor compose mail, a toolbar is missing.


Actual results:

The javascript console shows this error when trying to load the mail interface:

Error: NS_ERROR_XPC_JS_THREW_STRING: 'Permission denied to add https://us-mg5.mail.yahoo.com/neo/launch?action=compose&To=%s as a protocol handler' when calling method: [nsIWebContentHandlerRegistrar::registerProtocolHandler]
Source file: https://s.yimg.com/zz/combo?nq/s/launch/common-neo-base_54_8396.js&nq/s/launch/inbox-conversations_54_8396.js&nq/s/launch/inbox-mail-new_54_8396.js&nq/s/launch/inbox-base_54_8396.js&nq/s/intl/js/launch/lang_en-US_54_8396.js&nq/s/intl/js/launch/conf_us_54_8396.js
Line: 70


This is the problematic code:

if(window.navigator.registerProtocolHandler){
  var Bi=window.location.origin+"//neo/launch?action=compose&To=%s";
  window.navigator.registerProtocolHandler("mailto",Bi,"Yahoo!");
}

The problem is that by default Seamonkey doesn't have network.protocol-handler.external.mailto set to true (it doesn't even exist) - and if it's false or does not exist then window.navigator.registerProtocolHandler("mailto", ...) results in a javascript error and script execution is stopped. In the case of Yahoo Mail it breaks the page completely and makes it unusable.

registerProtocolHandler("mailto", ...) works only when the above pref is set to true, but this is a non-standard setting and most people don't use it.


Expected results:

It could be argued whether Yahoo should wrap the registerProtocolHandler() call in a try...catch block to fix the problem, but I think Seamonkey should still work without it because it's the only browser that has this function blocked due to being integrated with its own email client. Therefore, if a check for the existence of window.navigator.registerProtocolHandler is positive then it should be executed without errors, which is what the author of this script logically assumed.

The solution I suggest is to change the behaviour of registerProtocolHandler("mailto", ...) when network.protocol-handler.external.mailto is not true so that it doesn't produce a js error but only sends a warning to the console without stopping js execution.

This is important for Yahoo Mail users but may also be important for other web based mail systems as new ones can be soon created that will also want to use registerProtocolHandler() function. For best compatibility with Firefox this should be addressed so that when people test web apps in Firefox then they can (almost) safely assume they will also work in Seamonkey with the same rendering engine.

-----------------------------------------------------
I'm not sure whether this bug doesn't really belong to the core of the gecko engine because this same problem applies to Firefox. The only difference, but critical in this case, is that in Firefox network.protocol-handler.external.mailto is set to true by default (and there's really no need for anyone to change it) while in Seamonkey it's false (in fact it doesn't exist). Therefore, it's very important for this to be addressed in Seamonkey but because this behaviour is common between the two browsers then maybe it would make more sense for the fix to be applied to the core of both products. I'll leave this for the devs to decide.
Severity: normal → major
OS: Windows 7 → All
Hardware: x86 → All
FWIW, Firefox has network.protocol-handler.external.mailto set to true - but then, they don't have their own mail/news component as SeaMonkey does.

To clarify, "the mail app is almost unusable - can't read nor compose mail, a toolbar is missing" refers to SeaMonkey, not to the Yahoo site? Does the Yahoo site behave correctly and SeaMonkey's mail/news component behave correctly when network.protocol-handler.external.mailto is set to false before invoking the Yahoo script for the first time?
> 'Permission denied to add https://us-mg5.mail.yahoo.com/neo/launch?action=compose&To=%s as a protocol handler'

(on a side note, I'd consider this the correct behavior given that an arbitrary website shouldn't be able to mess with the protocol-handling settings...)
Dupe of bug 995706?
Ok, so per http://forums.mozillazine.org/viewtopic.php?f=38&t=2820341 instructions for FF you are supposed to be asked for permissions if you want to register Yahoo as protocol handler, this makes sense now. Thus, the difference is network.protocol-handler.external.mailto not being present at all (SeaMonkey default) vs. set to false vs. set to true (Firefox default).

Looking at the implementation in http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/src/nsIOService.cpp#402 it appears that a handler pref not being present defaults to false anyway. Do you see any difference between the missing and initialized-to-false cases?

The error message from [nsIWebContentHandlerRegistrar::registerProtocolHandler] you see in the Error Console may simply be the log result from the denial of that protocol to be registered, without prompting the user as Firefox does.

therube: Setting network.protocol-handler.external.mailto;true by default would prevent SM's own mail/news component from starting on a mailto: link, correct? Thus, if added to the SM preferences it could only be "false" by default to avoid that, but that wouldn't change the behavior as described, assuming that I'm reading the code handling that pref correctly.
Flags: needinfo?(therubex)
> Setting network.protocol-handler.external.mailto;true by default would prevent
> SM's own mail/news component from starting on a mailto: link, correct?

That does not look to be the case.

> network.protocol-handler.external.mailto;true

With that, Yahoo (new) web mail works properly, & mailto: links still prompt (in my case) to load Eudora.

> mailto:someone@example.com
Flags: needinfo?(therubex)
(In reply to therube from comment #5)
> > Setting network.protocol-handler.external.mailto;true by default would prevent
> > SM's own mail/news component from starting on a mailto: link, correct?
> 
> That does not look to be the case.

It does prevent it for me (on Linux) - creating network.protocol-handler.external.mailto as true and then clicking on the mailto: link in your comment opens the "Launch Application" dialog, thus trying to find an external application rather than using the internal component.

> With that, Yahoo (new) web mail works properly, & mailto: links still prompt
> (in my case) to load Eudora.

Which makes sense in your case as you are using SeaMonkey like Firefox as a browser only and don't use the internal mail/news component.

Thus, I don't think that network.protocol-handler.external.mailto;true is a viable options as a default value, but it could/should be defined in the SeaMonkey-specific prefs so that it is more easily available for modification. With network.protocol-handler.external.mailto;false the mail/news component is opened as expected.

If despite setting network.protocol-handler.external.mailto as false the Yahoo web app still won't function correctly it looks like a problem in their implementation to me as they should catch the case that registering the protocol handler is denied by the browser.
(In reply to rsx11m from comment #1)
> FWIW, Firefox has network.protocol-handler.external.mailto set to true - but
> then, they don't have their own mail/news component as SeaMonkey does.

Yes. Actually, I've tested Firefox with Yahoo as well and it appears that Firefox and Seamonkey work exactly the same in respecting the network.protocol-handler.external.mailto pref. The only difference is that in Firefox it's set to true by default and in Seamonkey it doesn't exist (which is equivalent to being set to false).

I can reproduce the exact same bug in Yahoo mail in Firefox when I set network.protocol-handler.external.mailto to false.

> To clarify, "the mail app is almost unusable - can't read nor compose mail,
> a toolbar is missing" refers to SeaMonkey, not to the Yahoo site?

This refers to Yahoo web mail that is opened in the Seamonkey browser. This has nothing to do with Seamonkey Mail component.

> Does the
> Yahoo site behave correctly and SeaMonkey's mail/news component behave
> correctly when network.protocol-handler.external.mailto is set to false
> before invoking the Yahoo script for the first time?

Well, the mail/news component has nothing really to do with this bug (I suppose) and should work regardless but when network.protocol-handler.external.mailto is set to false (or does not exist) then the Yahoo script in its web mail is broken.

(In reply to rsx11m from comment #3)
> Dupe of bug 995706?

Yes, certainly! I created this bug because the title of that one was misleading.

(In reply to rsx11m from comment #4)
> Ok, so per http://forums.mozillazine.org/viewtopic.php?f=38&t=2820341
> instructions for FF you are supposed to be asked for permissions if you want
> to register Yahoo as protocol handler, this makes sense now. Thus, the
> difference is network.protocol-handler.external.mailto not being present at
> all (SeaMonkey default) vs. set to false vs. set to true (Firefox default).

Yep!

> Looking at the implementation in
> http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/src/
> nsIOService.cpp#402 it appears that a handler pref not being present
> defaults to false anyway. Do you see any difference between the missing and
> initialized-to-false cases?

No difference at all.

> The error message from
> [nsIWebContentHandlerRegistrar::registerProtocolHandler] you see in the
> Error Console may simply be the log result from the denial of that protocol
> to be registered, without prompting the user as Firefox does.

Yes, and to me it appears be exactly that - the denial to be registered.

(In reply to rsx11m from comment #6)
> Thus, I don't think that network.protocol-handler.external.mailto;true is a
> viable options as a default value, but it could/should be defined in the
> SeaMonkey-specific prefs so that it is more easily available for
> modification. With network.protocol-handler.external.mailto;false the
> mail/news component is opened as expected.

I also don't think that setting network.protocol-handler.external.mailto to true by default is the option because then Seamonkey won't open its mail component for mailto links. It is only viable for those who use an external email client.

> If despite setting network.protocol-handler.external.mailto as false the
> Yahoo web app still won't function correctly it looks like a problem in
> their implementation to me as they should catch the case that registering
> the protocol handler is denied by the browser.

And so we've arrived at the crux of the matter! However, the problem is that how this should work is open to interpretation so let's discuss it.

The question is does throwing an exception by Seamonkey is the correct behaviour? The specs at http://www.whatwg.org/specs/web-apps/current-work/#custom-handlers say this:

"User agents may, within the constraints described in this section, do whatever they like when the methods are called. A UA could, for instance, prompt the user and offer the user the opportunity to add the site to a shortlist of handlers, or make the handlers his default, or cancel the request. UAs could provide such a UI through modal UI or through a non-modal transient notification interface. UAs could also simply silently collect the information, providing it only when relevant to the user."

To me this means that Seamonkey does not have to throw an exception as it can "do whatever it likes" and it can simply "cancel the request". Currently, it also "does what it likes" by throwing an exception but perhaps we should consider which behaviour will make more sense to the users without creating problems?

Throwing an exception certainly creates problems for Yahoo users by making their web mail inaccessible. Yahoo could fix their script by trying to catch the exception but will they do it? Seamonkey being such a niche browser we can't expect all developers to test sites in it so I think we should strive for as much compatibility with Firefox as possible. And Seamonkey is probably the only browser out there in which the problem can occur because no other current browser is integrated with an email client - so a try...catch block around registerProtocolHandler() is probably unnecessary for any browser except Seamonkey. We should also be aware that more and more web applications can be created that will want to utilize registerProtocolHandler() function and Seamonkey may again end up incompatible.

So my suggestion is this:

1. If network.protocol-handler.external.mailto is true then things can work like they currently do, no problem here.
2. If network.protocol-handler.external.mailto is false or doesn't exist, then let registerProtocolHandler() simply "cancel the request" without throwing an exception - optionally, a js warning could be sent to the console. Then Yahoo web mail will work again and potentially other web applications using registerProtocolHandler() that have not been tested in Seamonkey.

Is there anything that could be broken with this solution? In my opinion such a behaviour is still within the specs and I can't see how any harm could be done to any scripts by doing so.

I'm not saying that this solution is the only correct one or the best - my point is we shouldn't rely here on trying to evangelize Yahoo and others and instead we should strive for compatibility by default. Registering a mailto protocol by a web site is really a minor convenience feature, nothing too important, and we shouldn't have inaccessible sites due to such trivia. Let's make Seamonkey more compatible with sites that have been tested in Firefox.
(In reply to lemon_juice from comment #7)
> 1. If network.protocol-handler.external.mailto is true then things can work
> like they currently do, no problem here.

This is the current Firefox behavior, no problem for it indeed as it doesn't need to take into account any internal mail/news component. For SeaMonkey, this would be the optional behavior if the user prefers an external handler over the internal component, but can't be the default.

> 2. If network.protocol-handler.external.mailto is false or doesn't exist,
> then let registerProtocolHandler() simply "cancel the request" without
> throwing an exception - optionally, a js warning could be sent to the
> console. Then Yahoo web mail will work again and potentially other web
> applications using registerProtocolHandler() that have not been tested in
> Seamonkey.

Makes sense, but that change is no longer within the scope of SeaMonkey. That's a Core change, thus needs to be moved there.
Blocks: 995706
Status: UNCONFIRMED → NEW
Component: General → Networking
Ever confirmed: true
Product: SeaMonkey → Core
Summary: registerProtocolHandler("mailto") terminates js with error and breaks Yahoo Mail → registerProtocolHandler("mailto") terminates js with error and breaks Yahoo Mail [don't throw an exception in nsIWebContentHandlerRegistrar if protocol is to be handled internally]
Version: SeaMonkey 2.25 Branch → 28 Branch
So I take it things also work "correctly" in a Private Browsing window?

If sites aren't expecting registerProtocolHandler to throw, maybe we should just log the denial like we do for private browsing windows?
Flags: needinfo?(bzbarsky)
(In reply to neil@parkwaycc.co.uk from comment #9)
> So I take it things also work "correctly" in a Private Browsing window?

With SeaMonkey 2.26 Beta 1 (OS X), Yahoo! Mail doesn't work in a private browsing window, either.
Trying to use the webmail in a private browsing window I get a "Service is not defined" error at line 373 of source file resource://gre/components/WebContentConverter.js, don't know if that matters.
(In reply to Andrea Govoni from comment #10)
> Trying to use the webmail in a private browsing window I get a "Service is not defined"

That sounds like a different issue, not even getting to the point where setting the protocol handler is attempted when in private-browsing mode.
Per spec, registerProtocolHandler should never throw.  So yes, we should just log and move on.
Flags: needinfo?(bzbarsky)
(In reply to rsx11m from comment #11)
> (In reply to Andrea Govoni from comment #10)
> > Trying to use the webmail in a private browsing window I get a "Service is not defined"
> 
> That sounds like a different issue, not even getting to the point where
> setting the protocol handler is attempted when in private-browsing mode.

Actually it's just a typo introduced by bug 888310.
Attached patch Proposed patchSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8407365 - Flags: review?(iann_bugzilla)
Component: Networking → Feed Discovery and Preview
Product: Core → SeaMonkey
Version: 28 Branch → SeaMonkey 2.22 Branch
Comment on attachment 8407365 [details] [diff] [review]
Proposed patch

[Triage Comment]
r/a=me
Attachment #8407365 - Flags: review?(iann_bugzilla)
Attachment #8407365 - Flags: review+
Attachment #8407365 - Flags: approval-comm-beta+
Attachment #8407365 - Flags: approval-comm-aurora+
Neil, any reason not to land this so that users can test it?
Flags: needinfo?(neil)
User agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 SeaMonkey/2.25
Build identifier: 20140318183302

Same results as rsx11m in Comment#6 (It does prevent it for me (on Linux) - creating network.protocol-handler.external.mailto as true and then clicking on the mailto: link in your comment opens the "Launch Application" dialog, thus trying to find an external application rather than using the internal component.).

As a temporary workaround I've added a network.protocol-handler.external.mailto button on my prefbar so that I can toggle to true/false. 

I've found that once the user is logged into Yahoo! WebMail, the pref can be turned back off (false) and I'm then able to use Yahoo WebMail and standard mailto (example: mailto:support-seamonkey@lists.mozilla.org) without having to define SeaMonkey mailer as an external application.
NoOp, thanks for the comment, but by now it's known what causes the interoperability issue with the Yahoo webmail site and the patch approved here will fix it from SeaMonkey's side (it's only waiting to be checked in and will hopefully be available with the 2.26 release next week). The workaround with setting the network.protocol-handler.external.mailto preference should no longer be needed after that.
Pushed comm-central changeset 60dccdfc1277.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(neil)
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.28
Pushed comm-beta changeset 7a0f6238a786.

Bah, I just noticed I put the wrong a= on those changesets.
You need to log in before you can comment on or make changes to this bug.