Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

jQuery-UI 1.13.0 tooltip bug #1990

Closed
josepsanzcamp opened this issue Oct 8, 2021 · 10 comments · Fixed by #1994
Closed

jQuery-UI 1.13.0 tooltip bug #1990

josepsanzcamp opened this issue Oct 8, 2021 · 10 comments · Fixed by #1994
Labels
Behavior different from 1.12 Bug Comp: Tooltip Comp: Widget Has Pull Request
Milestone
1.13.1

Comments

@josepsanzcamp
Copy link
Contributor

Hi team.

I have found an issue in the latest release (1.13.0) when my self content function of the tooltip plugin returns an empty string, the problem not appears at the same moment but appear when you try to do something in other widgets of the user interface.

As an example, I have write the proof of concept that creates a datepicker after programs the tooltip plugin, if you use the previous month button, all works fine but when you move the mouse to the next month button, the problem appear and when you try to do a click in the button, a javascript error appear.

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <title>TEST</title>
    <link href="jquery-ui-1.13.0/jquery-ui.css" rel="stylesheet">
</head>
<body>
    <script src="jquery-ui-1.13.0/external/jquery/jquery.js"></script>
    <script src="jquery-ui-1.13.0/jquery-ui.js"></script>

<script>

"use strict";

$(function() {

    $(document).tooltip({
        items:"[title][title!='']",
        content:function() {
            var title = $(this).attr("title");
            if (title == "Next") {
                return "";
            }
            return title;
        }
    });

    $("body").append("<div id='datepicker'></div>");
    $("#datepicker").datepicker();

});

</script>

</body>
</html>

Thanks in advance.

Josep.
@caugner
Copy link
Contributor

caugner commented Oct 8, 2021

Can you confirm whether this was working in the previous version (1.12.1)?

@josepsanzcamp
Copy link
Contributor Author

Yes, the example works with 1.12.1 but not with 1.13.0

@caugner
Copy link
Contributor

caugner commented Oct 8, 2021 •
edited

Yes, the example works with 1.12.1 but not with 1.13.0

Thanks!

when you try to do a click in the button, a javascript error appear.

Can you post that error here?

@josepsanzcamp
Copy link
Contributor Author

The error that appear in the javascript console is:

14:52:11.435 Uncaught TypeError: this._find(...) is null
    jQuery 45
jquery-ui.js:18874:31
    jQuery 45

@caugner
Copy link
Contributor

caugner commented Oct 8, 2021

Thanks, this is the code location causing this error:

jquery-ui/ui/widgets/tooltip.js

Lines 351 to 357 in 70dae67

// Only bind remove handler for delegated targets. Non-delegated
// tooltips will handle this in destroy.
if ( target[ 0 ] !== this.element[ 0 ] ) {
events.remove = function() {
this._removeTooltip( this._find( target ).tooltip );
};
}

this._find( target ) returns null, so the tooltip function cannot be called on the return value.

However, that tooltip.js hasn't been changed since 1.12.1.

@josepsanzcamp Could you verify whether the issue also affects jQuery UI 1.13.0 with other major versions of jQuery, e.g. 1.9.1 or 2.2.4?

@josepsanzcamp
Copy link
Contributor Author

Hi @caugner.

I have tested that jquery 1.9.1, jquery 1.12.4 and jquery 2.2.4 works correctly with jquery-ui 1.12.1 but fails with jquery-ui 1.13.0.

Josep.

@josepsanzcamp
Copy link
Contributor Author

Hi @caugner.

I have detected that the problem is in the widget.js.

I recovered the try catch found inside the loop of the $.cleanData and seems that works.

Original code in the 1.13.0:

$.cleanData = ( function( orig ) {
	return function( elems ) {
		var events, elem, i;
		for ( i = 0; ( elem = elems[ i ] ) != null; i++ ) {

			// Only trigger remove when necessary to save time
			events = $._data( elem, "events" );
			if ( events && events.remove ) {
				$( elem ).triggerHandler( "remove" );
			}
		}
		orig( elems );
	};
} )( $.cleanData );

And the merged code using the try catch found in the 1.12.1 and the latest 1.13.0:

$.cleanData = ( function( orig ) {
	return function( elems ) {
		var events, elem, i;
		for ( i = 0; ( elem = elems[ i ] ) != null; i++ ) {
			try {

				// Only trigger remove when necessary to save time
				events = $._data( elem, "events" );
				if ( events && events.remove ) {
					$( elem ).triggerHandler( "remove" );
				}
			// Http://bugs.jquery.com/ticket/8235
			} catch ( e ) {}
		}
		orig( elems );
	};
} )( $.cleanData );

Does it help you???

@mgol
Copy link
Member

mgol commented Oct 8, 2021

@josepsanzcamp Thanks for the report. Nice find! The issue, as I see, is that the remove handler can throw when this._find( target ) returns null and it's no longer wrapped in try-catch when called from cleanData.

Would you like to submit a PR with a fix?

@mgol mgol added this to the 1.13.1 milestone Oct 8, 2021
@josepsanzcamp josepsanzcamp mentioned this issue Oct 8, 2021
Merged
@josepsanzcamp
Copy link
Contributor Author

Hi @mgol

I did a PR to recover the old try..catch, it's ok???

Thanks.

Josep.

@mgol mgol closed this as completed in #1994 Nov 8, 2021
mgol added a commit that referenced this issue Nov 8, 2021
@josepsanzcamp @caugner @mgol
Tooltip: Don't crash on empty content
85fba3f 
Commit 1f2011e removed a `try-catch` around triggering the `remove` handlers
in the `jQuery.cleanData` override. The `try-catch` was meant for old IE but it was
also catching an error coming from the tooltip `remove` handler depending on
being able to find a relevant tooltip. The `_find` method returns `null`, though,
when the tooltip cotent is empty.

Instead of restoring the `try-catch`, handle the `null` case in the `remove` handler.

Fixes gh-1990
Closes gh-1994

Co-authored-by: Claas Augner <github@caugner.de>
Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com>
@mgol
Copy link
Member

mgol commented Jan 20, 2022

jQuery UI 1.13.1 including a fix for this issue has been released: https://blog.jqueryui.com/2022/01/jquery-ui-1-13-1-released/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Behavior different from 1.12 Bug Comp: Tooltip Comp: Widget Has Pull Request
Milestone
1.13.1
Development

Successfully merging a pull request may close this issue.

Tooltip: Don't crash on empty content
3 participants
@josepsanzcamp @caugner @mgol