If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

fireEvent should report exceptions using Components.reportError

RESOLVED FIXED

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Nickolay_Ponomarev, Assigned: Joey Minta)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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>
(Assignee)

Comment 1

12 years ago
Created attachment 220315 [details] [diff] [review]
patch v1

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?
(Assignee)

Updated

12 years ago
Attachment #220315 - Flags: first-review? → first-review?(mconnor)

Updated

12 years ago
Attachment #220315 - Flags: first-review?(mconnor) → first-review+
(Assignee)

Comment 2

12 years ago
checked in on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
*** Bug 342763 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

10 years ago
Blocks: 396307
You need to log in before you can comment on or make changes to this bug.