Bug Tracker

Opened 7 years ago

Closed 6 years ago

#12374 closed bug (patchwelcome)

bug in jquery event function (since 1.4.3 version)

Reported by: requa@… Owned by:
Priority: high Milestone: None
Component: event Version: 1.8.0
Keywords: Cc: scott_gonzalez
Blocked by: Blocking:

Description

This is undiscovered bug in JQuery core since 1.4.3 version and up to now. Buggy function "e.returnValue = false;" can be found in non-minified version of latest JQuery, under "otherwise set the returnValue property of the original event to false (IE)". I mean, that in JQuery before 1.4.3 this line wasn't in brackets, but since this version of JQuery this code has been included into IF STATEMENT: "if ( e.preventDefault )", and redefined as the mismatch (else section) of this IF STATEMENT.

I have done many experiments with followed code. Many of JQuery UI bugs caused by this JQuery small error, that comes from 1.4.3 version. And all of them disappeared, when I edit this line.

Can you do deep revision of this section and return everything back to the code from JQuery 1.4.3 (exact copy): "

if preventDefault exists run it on the original event if ( e.preventDefault ) { e.preventDefault(); } otherwise set the returnValue property of the original event to false (IE) e.returnValue = false;

"

Contact to me via email REQUA@… for details if needed.

Change History (10)

comment:1 Changed 7 years ago by dmethvin

Owner: set to requa@…
Status: newpending

Hi, I'm not sure why you are calling that buggy. Can you provide a test case that shows some incorrect behavior as a result? Please use jsfiddle.net or jsbin.com if possible so we can experiment with your test case.

comment:2 Changed 6 years ago by requa@…

Status: pendingnew

Hi, I've posted my test script on jsfiddle.net. http://jsfiddle.net/JLpby/ First of all, I'm talking about demonstration of this bug in Google Chrome. When the page loads completly, try to resize iframe by dragging rigth bottom corner (this is small drag icon). It resizes very very softly, without any "stops" during very long time. But when you change version of loading JQuery from 1.4.2 to 1.4.3, iframe will stop to resize softly and the bug will become applied. It happens because of the bug described in this ticket. You may take JQuery up to 1.7.2 version and change this line of code, and strange bug disappears in Google Chrome, as I did it.

But in Opera this no bug at all, since oldest version of JQuery and up to latest. In Firefox it was buggy everytime and I don't know the reason of it.

Replying to dmethvin:

Hi, I'm not sure why you are calling that buggy. Can you provide a test case that shows some incorrect behavior as a result? Please use jsfiddle.net or jsbin.com if possible so we can experiment with your test case.

comment:3 Changed 6 years ago by dmethvin

Status: newpending

Sorry but I don't see any difference in behavior in Chrome. Here is the one I used for comparison: http://jsfiddle.net/dmethvin/JLpby/1/

The e.returnValue = false; should not be needed or executed by anything other than IE 6/7/8.

Can you provide some more information about why you think *not* executing that line is causing a problem in Chrome?

comment:4 Changed 6 years ago by requa@…

Status: pendingnew

I can see the same problem and bug in your example. During long-time resizing (>5-10 secs, horizontal and vertical in same time), border drawing stops and lags very much. But there are no stops and lags in my example in latest Chrome.

http://jsfiddle.net/JLpby/

As I tested, during rewriting JQuery Core from 1.4.3 up to 1.7.2, Chrome needs "e.returnValue = false;" somehow. But this is something else in JQuery 1.8.0 that also causes this bug... I'll figure it out, but first of all, the solution of discovered bug is required.

comment:5 Changed 6 years ago by mikesherov

Component: unfiledevent
Owner: changed from requa@… to dmethvin
Priority: undecidedhigh
Status: newassigned

Confirmed.

Last edited 6 years ago by mikesherov (previous) (diff)

comment:6 Changed 6 years ago by dmethvin

Cc: scott_gonzalez added

The change was made here, September 20, 2010:

https://github.com/jquery/jquery/commit/be59693037c4230e1b395e51a42c6fc55b577455

So yes that would correspond with the problem occurring with 1.4.3 which was released in October 2010. Although the commit message doesn't mention it, I could swear that I had a conversation with Scott Gonzalez about this very thing at the time and there was a bug being *caused* by the e.returnValue being there in some non-IE browser. He figured it out using git bisect and I was suitably impressed. Maybe related to this UI bug which was fixed around the same time? http://bugs.jqueryui.com/ticket/6174

Chrome needs e.returnValue = false; somehow

That's *really* puzzling, since e.preventDefault() should be all Chrome needs to do its job. Total guess, but perhaps there is an underlying Chrome bug regarding the iframe and some security issue is preventing it from calling the .preventDefault() method properly. Notice that there are several security warnings being thrown from within the iframe when it loads.

Using the Chrome debugger I have changed the jquery-git.js code to add event.returnValue = false; and the test case behaves the same. So I am not sure this is related to the cause.

comment:7 Changed 6 years ago by scottgonzalez

The bugginess in resizing an iframe is caused by calling event.preventDefault() on mousedown. jQuery UI <= 1.8.4 didn't do this in Chrome because .disableSelection() used the unselectable attribute and resizable itself only called event.preventDefault() in non-WebKit browsers. jQuery UI 1.8.5 has the bug because .disableSelection() was changed to use mousedown and selectstart events. If you use jQuery UI 1.8.5 and jQuery 1.4.3 and make .disableSelection() a noop, the bug will go away. However, in jQuery UI 1.8.6 I removed the conditional from resizable and now event.preventDefault() is called in all browsers.

Setting event.returnValue must be triggering some strange state where the default behavior is prevented but the behavior changes. This is certainly a strange bug.

comment:8 Changed 6 years ago by mikesherov

related to #11806 ?

comment:9 Changed 6 years ago by dmethvin

Owner: dmethvin deleted
Status: assignedopen

comment:10 Changed 6 years ago by dmethvin

Resolution: patchwelcome
Status: openclosed

This is some kind of incredibly rare corner case, and when I added the event.returnValue it didn't change the behavior. Patch welcome.

Note: See TracTickets for help on using tickets.