Last Comment Bug 233142 - onmouseover that doesn't return prevents URLs from appearing in status bar
: onmouseover that doesn't return prevents URLs from appearing in status bar
Status: RESOLVED FIXED
: regression, testcase
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- major (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
Mentors:
http://www.neowin.net/forum/index.php...
Depends on: 132777
Blocks: 134878
  Show dependency treegraph
 
Reported: 2004-02-05 03:10 PST by Jesse Ruderman
Modified: 2004-03-08 15:28 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (182 bytes, text/html)
2004-02-05 03:11 PST, Jesse Ruderman
no flags Details
Fix, and take care of not submitting if onsubmit fails too (9.98 KB, patch)
2004-02-13 17:11 PST, Johnny Stenback (:jst, jst@mozilla.com)
bzbarsky: review+
bryner: superreview+
Details | Diff | Splinter Review

Description Jesse Ruderman 2004-02-05 03:10:08 PST
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040203
Firebird/0.8.0+

Steps to reproduce:
1. Load the testcase.
2. Hover over the link.
Result: no URL appears in the status bar.

The testcase has an onmouseover that doesn't do or return anything:
javascript:document.onmouseover = function() { };

Regression window (testing Firebird trunk):
good: 12/16
broken: 12/31
broken: 02/03

Related: bug 227159 (but that was filed Dec 1, and this regressed after Dec 16.)

This bug is not present on the Firebird 0.8 branch.
Comment 1 Jesse Ruderman 2004-02-05 03:11:44 PST
Created attachment 140661 [details]
testcase
Comment 2 Boris Zbarsky [:bz] 2004-02-05 10:17:36 PST
Any chance of a smaller (like 1-day) regression window?
Comment 3 Hermann Schwab 2004-02-05 12:31:05 PST
tested on Mozilla Suite Win98, 
BuildID 2003121908 is working, 
BuildID 2003122308 is failing.

After loading has finished, 'done' is seen in the statusbar.
If you hover over the link, 'done' is cleared, so the hover is recognized, but
the URL not written to the status bar.
Comment 4 Boris Zbarsky [:bz] 2004-02-05 12:46:57 PST
This is a regression from bug 226462.  The old logic was:

keep_going = "not bool returned" || (reverse ? !return : return);

The new logic is:

keep_going = ("not bool returned" || return) xor (reverse)

So in the old code, if the retval was not a bool, we kept going no matter what
retval happened to be or what reverse was.  In the new code, if the retval is
not a bool and reverse is true we kill the event.  In the testcase here, I
expect the retval is undefined or some such?
Comment 5 Boris Zbarsky [:bz] 2004-02-13 13:24:48 PST
Note that you've also changed the behavior when the handler throws an exception.
 Prior to the patch for bug 226462 that would end up returning true (since "ok"
would be false) and such handlers would not cancel the event.   In the current
code, an exception in a handler will cancel an event.

Marking major due to that -- that's a pretty severe change in behavior,
especially if we don't intend it.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2004-02-13 14:30:51 PST
Duh, I apparently missed your comment in bug 226462. Looking.
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2004-02-13 17:11:15 PST
Created attachment 141371 [details] [diff] [review]
Fix, and take care of not submitting if onsubmit fails too

This should revert to the old logic (with some minor differences in error
cases, but I seriously doubt that matters), and it also makes us prevent the
default action of onsubmit events if there's any error in handling onsubmit
events.
Comment 8 Boris Zbarsky [:bz] 2004-02-13 17:25:35 PST
Comment on attachment 141371 [details] [diff] [review]
Fix, and take care of not submitting if onsubmit fails too

Makes sense.  r=bzbarsky
Comment 9 Jesse Ruderman 2004-02-21 16:17:45 PST
jst checked in a fix yesterday.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2004-02-22 14:24:19 PST
FIXED.

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