Last Comment Bug 226462 - Eliminate the aReverseReturnResult argument from nsJSContext::CallEventHandler()
: Eliminate the aReverseReturnResult argument from nsJSContext::CallEventHandler()
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.7alpha
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-11-21 11:51 PST by Johnny Stenback (:jst, jst@mozilla.com)
Modified: 2004-02-05 12:42 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Eliminate aReverseReturnResult (11.86 KB, patch)
2003-11-21 16:28 PST, Johnny Stenback (:jst, jst@mozilla.com)
caillon: review+
brendan: superreview+
Details | Diff | Splinter Review

Description Johnny Stenback (:jst, jst@mozilla.com) 2003-11-21 11:51:04 PST
It's silly, and messy, and only one caller cares so the argument should be
eliminated and the mess should be cleaned up. Patch coming up.
Comment 1 Johnny Stenback (:jst, jst@mozilla.com) 2003-11-21 16:28:59 PST
Created attachment 136075 [details] [diff] [review]
Eliminate aReverseReturnResult

Brendan, here's some spiffy XOR logic for you! I hope I got it right :-)
Comment 2 Brendan Eich [:brendan] 2003-11-21 16:44:13 PST
Comment on attachment 136075 [details] [diff] [review]
Eliminate aReverseReturnResult

Isn't this comment backward, because it describes the condition that's the
operand of !, not the condition that the if tests?

+    // if mReturnResult == nsReturnResult_eDoNotReverseReturnResult and
+    // jsBoolResult == true, or if mReturnResult !=
+    // nsReturnResult_eDoNotReverseReturnResult and jsBoolResult ==
+    // false, prevent default

Also, it's awfully "code-like" instead of saying in prose what is going on. 
Maybe "if the handler returned false and its sense is not reversed, or the
handler returned true and its sense is reversed from the usual (false means
cancel), then prevent default."

/be
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2003-11-21 17:55:27 PST
Yeah, duh, that comment was backwards. I replaced it with the wording you suggested.
Comment 4 Christopher Aillon (sabbatical, not receiving bugmail) 2003-12-06 16:42:15 PST
Comment on attachment 136075 [details] [diff] [review]
Eliminate aReverseReturnResult

Nifty tricks!  r=caillon with the comment update from brendan.
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2003-12-21 10:01:41 PST
Fixed.
Comment 6 neil@parkwaycc.co.uk 2003-12-21 16:10:35 PST
x == y is probably more readable than !(x ^ y)
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2004-02-05 12:42:10 PST
This caused bug 233142 

Note You need to log in before you can comment on or make changes to this bug.