628470 - popup.xml#popup-base, hidePopup method throws exceptions for failed QI to nsIMenuBoxObject

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Alex Vincent [:WeirdAl]]

This is extremely annoying for Venkman users.  Can we switch to instanceof, already?

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: switch to instanceof in showPopup, hidePopup

Comment 2

[Alex Vincent [:WeirdAl]]

Comment on attachment 506606 [details] [diff] [review]
switch to instanceof in showPopup, hidePopup

requesting approval for FF4:  Zero risk, and one less exception to be thrown and invisibly caught.

Comment 3

[Dão Gottwald [:dao]]

try {
  menuBox = this.parentNode.boxObject;
} catch(e) {}

When would this throw, i.e. why is the try/catch still needed?

Comment 4

[Alex Vincent [:WeirdAl]]

(In reply to comment #3)
> try {
>   menuBox = this.parentNode.boxObject;
> } catch(e) {}
> 
> When would this throw, i.e. why is the try/catch still needed?

I don't know if it would ever throw.  To my knowledge, it's only the QI calls that were throwing.

That said, I don't think we want to take a bigger change than absolutely necessary at this late stage.  The way it's written, if it did throw, menuBox would be null, and (null instanceof Components.interfaces.nsIMenuBoxObject) is always false.

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/a3a568807d82