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.

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

Fix detection of Fluid Mode #2224

Merged
merged 1 commit into from May 19, 2017

Conversation

lucash
Copy link
Contributor

@lucash lucash commented May 19, 2017

In Javascript the || operator does not necessarily return true or false,
but it returns the first value (if it is truthy) or the second value (if
the first value is falsey). This made this method return numbers instead
of booleans while the return value is compared to booleans with the
strict equal (===) operator which then gave wrong results.

In Javascript the || operator does not necessarily return true or false,
but it returns the first value (if it is truthy) or the second value (if
the first value is falsey). This made this method return numbers instead
of booleans while the return value is compared to booleans with the
strict equal (===) operator which then gave wrong results.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.832% when pulling 2701947 on lucash:fix-fluid-mode-detection into 5ddf254 on mediaelement:master.

@lucash
Copy link
Contributor Author

lucash commented May 19, 2017

The && operator works similar: It will return the first value if that is falsey, otherwise it will return the second value. Also see MDN: Logical Operators.

@rafa8626
Copy link
Contributor

Thanks for the info. I'll check the places where ~ is being used and make sure I use !== -1 instead to avoid this issue.

@rafa8626 rafa8626 merged commit 2701947 into mediaelement:master May 19, 2017
@rafa8626
Copy link
Contributor

PR merged. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants