312308 - tabbrowser: closing tab throws exception when no "autocompletepopup" attribute is set

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Garrett Davis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7 (ax)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7 (ax)

Using the tabbrowser widget without the "autocompletepopup" attribute set, will
cause an exception to be thrown in the "detachFormFill()" method of the browser
widget when a tab is closed.

When the tabbrowser is opening a tab, it creates a browser widget and copies
some if its attributes along to the browser widget. One attribute that it is
passing blindly is the "autocompletepopup" attribute. When the tabbrowser widget
doesn't have this attribute set, it will call setAttribute on the browser
passing the result of the getAttribute from itself. This will result in an
"autocompletepopup" attribute that exists in the browser widget, but is empty.
The browser widget's "attachFormFill()" method simply checks for the existence
of the attribute, tries to retrieve the element with an empty string for an ID,
fails, does not call nsIFormFillController::attachToBrowser(...), yet still sets
a flag indicating that it did call attachToBrowser.  When the tab is closed, the
browser widget tries to call nsIFormFillController::detachFromBrowser(...) which
throws an exception if attachToBrowser hasn't been called.

Reproducible: Always

Steps to Reproduce:
1. Write a XULRunner application that uses the tabbrowser widget.
2. Open a tab in the widget.
3. Close the tab.

Actual Results:  
Get the following debug messages in the console indicating an exception:

###!!! ASSERTION: getElementById(""), fix caller?: '!aId.IsEmpty()', file
d:/Mozilla/dev/trunk/mozilla/content/xul/document/src/nsXULDocument.cpp, line
1660 WARNING: NS_ENSURE_TRUE(index >= 0) failed, file
d:/Mozilla/dev/trunk/mozilla/toolkit/components/satchel/src/nsFormFillController.cpp,
line 195
*** uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIFormFillController.detachFromBrowser]" 
nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/bindings/browser.xml :: detachFormFill :: line 385" 
data: no]

Expected Results:  
No debug messages indicating an exception or failure.

Comment 1

[Garrett Davis]

Attachment: Patch for tabbrowser and browser widgets

Comment 2

[Benjamin Smedberg]

Comment on attachment 199413 [details] [diff] [review]
Patch for tabbrowser and browser widgets

In browser.xml please check for .hasAttribute("autocompletepopup") the same way
you do in tabbrowser.xml. Our internal code asserts if you try to do
getElementById(null).

Comment 3

[Garrett Davis]

(In reply to comment #2)
> (From update of attachment 199413 [details] [diff] [review] [edit])
> In browser.xml please check for .hasAttribute("autocompletepopup") the same way
> you do in tabbrowser.xml. Our internal code asserts if you try to do
> getElementById(null).
> 

The attachFormFill method in browser.xml already has the
.hasAttribute("autocompletepopup") check.  The getElementById() will never get
called if the attribute doesn't exist.  Are you referring to another location in
browser.xml?  If so, where?

Comment 4

[Benjamin Smedberg]

Comment on attachment 199413 [details] [diff] [review]
Patch for tabbrowser and browser widgets

Oh, I see.

Could you attach a version of this patch where the brace follows the if on the
same line (to match surrounding style).

Comment 5

[Garrett Davis]

Attachment: Patch with curly brackets at end of line

Comment 6

[Benjamin Smedberg]

Comment on attachment 199806 [details] [diff] [review]
Patch with curly brackets at end of line

very low-risk patch fixes uses of tabbrowser in xulrunner apps

Comment 7

[Benjamin Smedberg]

Fixed on trunk.

Comment 8

[Benjamin Smedberg]

Fixed on 1.8 branch.