> IsPreferred() is doing the wrong thing - it always returns false. I suspect that was the whole point of the change, in fact: the goal was to have link clicks in e-mails go and load in the browser, not in the messagepane. Does your patch not break that?
(In reply to comment #1) > > IsPreferred() is doing the wrong thing - it always returns false. > > I suspect that was the whole point of the change, in fact: the goal was to have > link clicks in e-mails go and load in the browser, not in the messagepane. > Does your patch not break that? IsPreferred doesn't seem to get called for links (generally in this set up). This appears to be the case whether or not network.protocol-handler.expose-all is true or false (note that false is the normal which means links go to the browser). The only case I've had it called is when showing the sign in to google calendar page and you click on the "Sign In" button. It does seem to be that network.protocol-handler.expose-all is the main factor for links in the message/browser elements.
> IsPreferred doesn't seem to get called for links (generally in this set up). In Thunderbird, or suite (where this code was initially added)? > note that false is the normal Not in suite.
Originally Mozilla relied on the URI content listener to divert all links and form submissions out of the message pane. A click handler was added because clicking on a link to a slow or resource could get cancelled if another message was read while the link was trying to load, although this did cause issues for links to downloads. URI content listeners also cannot divert error pages. Ideally I wouldn't want arbitrary content to load into a messagepane docshell for at least as long as there's no session history navigation.
Comment on attachment 407372 [details] [diff] [review] Proposed fix I'm going to switch the r/sr on this since Neil is much more aware of the issues and history here.
I've been doing a bit more work on this. Just changing IsPreferred (either via the patch or changing the return value) isn't enough. We also have to set network.protocol-handler.expose.http to true as well. This is because, for forms, we go through nsFormSubmission::SubmitTo -> nsWebShell::OnLinkClickSync. OnLinkClickSync checks with the external protocol handler to see if the protocol is exposed, and if it isn't, it redirects the link click to the external app rather than loading internally. For Thunderbird to set network.protocol-handler.expose.http to true, this would mean that clicking on links in the message pane would visit those links within the message pane - given that SeaMonkey doesn't do this, there may be something I'm missing. Investigating a bit more...
SeaMonkey has a click handler which looks for link clicks and diverts them to a browser window. Without that handler, a link click would try to load in the message pane; a form submit still does try, but the URIContentListener prevents it and it ends up loading in a browser window anyway.
Ok, so SeaMonkey's messagePaneOnClick() function handles the redirect of clicks on http etc to the main browser. So in theory we could set network.protocol-handler.expose.http to true (and any other appropriate prefs) and have our message pane on click handler redirect clicks to the external app handler service. The downside to this, is that we'd also need to provide some kind of default handler that toolkit can use - e.g. the links in add-on manager - something that hooks into nsIURILoader - which is an nsIURIContentListener which is what nsMsgWindow provides. I think there would be ways of moving this forward, but both would probably include limiting the existing registration of the content listener to the message pane docshell rather than the whole window. We may need to ifdef some code as well so that Thunderbird could handle things differently.
Created attachment 409706 [details] [diff] [review] Only apply content listener on message pane. Work in progress patch that's better that the last - with this we still register the content listener but just for the messagepane. This means that we'll do the right thing for messages, but allow other browser elements in the window to do what they want to do. I think I need more support items/work in here so waiting until I have done a bit more work before requesting review.
Comment on attachment 409706 [details] [diff] [review] Only apply content listener on message pane. Ok, this is the one I can go with. It moves the content listener so it only applies to the message pane docshell and not the whole window. This is sufficient for what I need - hence other browser elements in the mail window will be able to handle forms correctly. I tested this with a web form in an email and the behaviour was unchanged.
Checked in: http://hg.mozilla.org/comm-central/rev/1f8572a376ba http://hg.mozilla.org/releases/comm-1.9.1/rev/21540f0d022d