Closed Bug 309814 Opened 19 years ago Closed 18 years ago

fireEvent should report exceptions using Components.reportError

Categories

(Toolkit :: UI Widgets, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: asqueella, Assigned: jminta)

References

Details

Attachments

(1 file)

Various XBL bindings declare a function called _fireEvent or similar, which
fires the specified DOM event and executes the code in on*** attribute of the
target element.

In certain bindings (colorpicker, preferences) the function's body is wrapped in
a try..catch block, to make sure that an error/exception in the on*** handler
doesn't cause the fireEvent's caller to halt.

The problem is, in the catch clause the exception is dump()-ed to console, but
not to the JS Console. Many XUL developers use the JS Console for
troubleshooting, and it's confusing to them that some of  errors in their code
doesn't get reported to the JS Console. (The problem here is inconsistency -
<button>'s oncommand reports the error, while <prefpane>'s onpaneload does not.)

The solution is to report the error using Components.reportError(e) instead of
(or in addition to) dump(e). This will report the errors to the JS Console,
albeit with wrong location info.

--
To test it, you can load the following XUL (chrome):
<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>

<prefwindow id="test"
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" >
  <prefpane id="paneGeneral" label="General"
onpaneload="alert(0);undefinedFunction1();alert(1)">
    <button onclick="undefinedFunction2()"/>
  </prefpane>
</prefwindow>
Attached patch patch v1Splinter Review
Patch moves from dump() to Components.utils.reportError() for all the instances I could find of *fireEvent that included a try {} block.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #220315 - Flags: first-review?
Attachment #220315 - Flags: first-review? → first-review?(mconnor)
Attachment #220315 - Flags: first-review?(mconnor) → first-review+
checked in on trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
*** Bug 342763 has been marked as a duplicate of this bug. ***
Blocks: 396307
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: