Skip to content

Widget: fix handling of options === true under use strict in _show/_hide #1931

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.

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

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

Interghost
Copy link
Contributor

When using jQuery UI under use strict, options === true (default for ie tooltip show/hide) will throw
Uncaught TypeError: Cannot create property 'complete' on boolean 'true'
on line
options.complete = callback;

Fix for options === true when using jQuery UI under use strict, which throws 
Uncaught TypeError: Cannot create property 'complete' on boolean 'true'
on line
options.complete = callback;
@jsf-clabot
Copy link

jsf-clabot commented Jul 15, 2020

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@@ -717,6 +717,8 @@ $.each( { show: "fadeIn", hide: "fadeOut" }, function( method, defaultEffect ) {
options = options || {};
if ( typeof options === "number" ) {
options = { duration: options };
} else if ( options === true ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is true allowed for options? Is there a current test that fails without this? If not, could you add one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it was happening in tooltip with default show/hide option which is true
https://api.jqueryui.com/1.12/tooltip/#option-show
https://api.jqueryui.com/1.12/tooltip/#option-hide
this gets passed to the function.

Note that this is under "use strict" only, without use strict it just eats it up... I didn't run the tests, and I think adding one would be too much for me atm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a code example that shows the problem? It's still not clear to me how this would occur.

Copy link
Contributor Author

@Interghost Interghost Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://jsfiddle.net/fgLoq65n/
Note I had to paste the entire UI in the JS editor so it will take some time to load, just be patient... after load (when JS code gets colored) open console and mouseover the input box

Copy link
Member

@dmethvin dmethvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took a while, reading the code above these lines I agree this looks like a bug! The false case is handled elsewhere so only true trips it up.

Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 by reading

@fnagel fnagel merged commit ed94edd into jquery:master Oct 6, 2020
mgol pushed a commit that referenced this pull request Oct 6, 2020
Fix for `options === true` when using jQuery UI under `use strict`,
which throws:
```
Uncaught TypeError: Cannot create property 'complete' on boolean 'true'
```
on line:
```js
options.complete = callback;
```

Closes gh-1931
@mgol mgol added this to the 1.13 milestone Oct 15, 2020
@mgol mgol mentioned this pull request Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants