274300 - Hooks should not be able to break event processing

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[James Ross]

Currently hooks are pretty trusted things - throw a JS exception and the main
event loop in ChatZilla stops. V bad.

My proposal is to wrap the call to the hook's function in a try/catch, and if an
exception occurs use dd() to log the exception to the console, and *continue*
processing other hooks (unlike returning false).

Comment 1

[James Ross]

I've also noticed the return value of onHook is completely ignored! Perhaps that
should be fixed too, so hooks can /actually/ stop events.

Comment 2

[James Ross]

Attachment: Catch and handle exceptions in hooks

Comment 3

[Robert Ginda]

Comment on attachment 190305 [details] [diff] [review]
Catch and handle exceptions in hooks

I'd suggest you move the rv test outside of the try/catch block, so that only
"rv = hooks[h].f(e);" is left inside.

r=rginda with that change.

Comment 4

[James Ross]

Attachment: Take two

Comment 5

[Robert Ginda]

Comment on attachment 190340 [details] [diff] [review]
Take two

r=rginda

Comment 6

[James Ross]

Checked in --> FIXED.