287677 - 3 problems with Protocol Not Known Error (console error when dragged, tab overwrite, history/back functionality breaks.)

Status: RESOLVED FIXED

(Toolkit :: XUL Widgets, defect)

Description

[Daniel C]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050324 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050324 Firefox/1.0+

A. When dragging a URI with an unknown protocol an error appears in JavaScript
console.

B. When dragging a URI with an unknown protocol from one tab to a blank area on
the tab bar, or to another tab, both the new tab AND the tab that the URI was
dragged from display the "Protocol Not Know Error" page.

C. The Protocol Not Known Error breaks the history and back functionality.

Reproducible: Always

Steps to Reproduce:
A1. Drag the URL for this bug to the location bar or the tab title.

B1. Drag the URL for this bug (httpp://www.mozilla.org./) to a new tab, or to a
blank area on the tab-bar.

C1. Copy and paste this bug's URL into the location bar for this tab.
C2. Press back.
Actual Results:  
A. Error appears in JavaScript Console: 

Error: uncaught exception: [Exception... "Component returned failure code:
0x804b0012 [nsIWebNavigation.loadURI]"  nsresult: "0x804b0012 (<unknown>)" 
location: "JS frame :: chrome://global/content/bindings/browser.xml ::
loadURIWithFlags :: line 169"  data: no]

B. Both the current tab, and the new tab display the Protocol Not Known Error page.

C. Error page replaces current location, so back button skips the current page
and goes to the previous page (2 pages before the error page.)

Expected Results:  
A. No error.

B. Only the new tab should have displayed the error page.

C. Back button should only go back 1 page.

B & C: Dataloss? Current tab gets overwritten, location is as good as lost
(going back twice, then forwards once is not an acceptable way of returning to
the page.)

C: https://bugzilla.mozilla.org/show_bug.cgi?id=226401 and
https://bugzilla.mozilla.org/show_bug.cgi?id=157004 seemed to fix this problem
for all the other other error messages.

None of these problems seem to occur with other error pages.

Blocking https://bugzilla.mozilla.org/show_bug.cgi?id=28586 ?

Comment 1

[zug_treno]

I see these 3 bugs with Mozilla 1.8b1. When using a nightly (Mozilla 1.8b2)
without the error pages enabled I only get the 'httpp is not a registered
protocol' dialog twice and not the results from B and C.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I can't reproduce any of this, with a current trunk Firefox build or current
trunk SeaMonkey build.

Could you please give detailed steps to reproduce?  For _one_ problem?  Starting
with any changes from the default configuration that you have?

Comment 3

[Daniel C]

(In reply to comment #2)
> I can't reproduce any of this, with a current trunk Firefox build or current
> trunk SeaMonkey build.
> 
> Could you please give detailed steps to reproduce?  For _one_ problem?  Starting
> with any changes from the default configuration that you have?

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050416 Firefox/1.0+

Reproducable: Always

Steps to Reproduce:

0. Clean profile apart from:
   javascript.options.showInConsole = true
   javascript.options.strict = true
   browser.xul.error_pages.enabled = true
   (Restart browser after setting the last one.)
1. Open 2 brand new tabs (we'll call them 'Tab A' and 'Tab B', and have anything
but about:blank loaded in both of them.
2. In Tab A, type this into the location bar: hhttttpp://www.mozilla.org./ but
*don't* navigate to that URL.
3. Select all of that text, and drag it from the location bar in Tab A, onto Tab
B in the tab bar.
4. Open up the JavaScript Console (Tools > JavaScript Console.)
5. Attempt to navigate back to the original page that was in Tab A, or Tab B.

Actual Results:

After step 3, both tabs showed the Protocol Not Known Error (only the tab that
the URL was dragged to should have.)
After step 4, there was an error in the Console: 

Error: uncaught exception: [Exception... "Component returned failure code:
0x804b0012 [nsIWebNavigation.loadURI]"  nsresult: "0x804b0012 (<unknown>)" 
location: "JS frame :: chrome://global/content/bindings/browser.xml ::
loadURIWithFlags :: line 169"  data: no]

After step 5, the original page is difficult to navigate back to.

These steps work every time for me, and include all 3 problems in one, because I
actually think they are possibly the same problem.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So... problem 1 (error page shown in both tabs) is a totally separate issue from
problem 2 (exception in JS console).  There needs to be a separate bug on problem 2.

Problem 1 may or may not be related to problem 3; I'd file separate bugs on them.

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK, the inability to go back is long fixed.  The rest of this is SO not an
embedding problem.

The issue is that the drop handler in tabbrowser calls loadURI, which happily
throws on an unknown scheme.  Which means we never get to calling
event.stopPropagation(), which means we treat the drop as a drop into the
current browser once it bubbles up enough.

So the solution is either to try/catch around the nsDragAndDrop.drop call, or
around the loadURI call in that, or to make <browser>'s loadURI not throw. 
Frankly, I think I like this last option the most....

Comment 6

[[e-mail]]

(In reply to comment #5)
>So the solution is either to try/catch around the nsDragAndDrop.drop call, or
>around the loadURI call in that, or to make <browser>'s loadURI not throw. 
>Frankly, I think I like this last option the most....
Historically we've used the second option, as there might be callers who
otherwise would not know that the load will not succeed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

True, but is anyone actually checking whether loadURI throws anyway?  And how
would they tell if the load fails because of some other error (other than
unknown protocol)?

Maybe we should just change webnavigation to not throw the unknown protocol
error if it happens and be done with this.

Comment 8

[Joey Minta]

Attachment: patch v1

This patch takes the second approach, placing loadURI in a try/catch block.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 220318 [details] [diff] [review]
patch v1

>Index: toolkit/content/widgets/tabbrowser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v
>retrieving revision 1.146
>diff -p -U8 -r1.146 tabbrowser.xml
>--- toolkit/content/widgets/tabbrowser.xml	21 Apr 2006 14:56:42 -0000	1.146
>+++ toolkit/content/widgets/tabbrowser.xml	30 Apr 2006 17:15:41 -0000
>@@ -1623,17 +1623,22 @@
> 
>               if (document.getBindingParent(aEvent.originalTarget).localName != "tab") {
>                 // We're adding a new tab.
>                 this.loadOneTab(getShortcutOrURI(url), null, null, null, bgLoad, false);
>               }
>               else {
>                 // Load in an existing tab.
>                 var tab = aEvent.target;
>-                this.getBrowserForTab(tab).loadURI(getShortcutOrURI(url));
>+                try {
>+                  this.getBrowserForTab(tab).loadURI(getShortcutOrURI(url));
>+                } catch(ex) {
>+                  // Bail on invalid urls
>+                  return;
>+                }
> 
>                 if (this.mCurrentTab != tab && !bgLoad)
>                   this.selectedTab = tab;

cleaner option is probably to just move the bit after the catch inside of the try block and drop the return in the catch block, r=me with that changed.

Comment 10

[Joey Minta]

Checking in toolkit/content/widgets/tabbrowser.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v  <--  tabbrowser.xml
new revision: 1.155; previous revision: 1.154
done