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

Don't rely on {}.toString() in old browsers (IE<=8) #185

Closed
wants to merge 1 commit into from
Closed

Don't rely on {}.toString() in old browsers (IE<=8) #185

wants to merge 1 commit into from

Conversation

oleg8sh
Copy link
Contributor

@oleg8sh oleg8sh commented Aug 7, 2014

As you know, toString() method called on null returns "[object Null]", and on undefined returns "[object Undefined]".
But this is true for ECMAScript 5.1 and above.

Older browsers in both cases returns window.toString() that is "[object Object]".

So m("br") fails in IE8 when it tries to get hasAttrs variable value.

The same troubles can be found in build() if cached is undefined.

More info: http://tobyho.com/2011/01/28/checking-types-in-javascript/

P.S.: Changes in line 37 "... cached === null || cached === undefined ..." is also an alternative solution of #162
( #162 )

As you know, toString() method called on null returns "[object Null]", and on undefined returns "[object Undefined]".
But this is true for ECMAScript 5.1 and above.

Older browsers in both cases returns window.toString() that is "[object Object]".

So m("br") fails in IE8 when it tries to get hasAttrs variable value.

The same troubles can be found in build() if cached is undefined.

More info: http://tobyho.com/2011/01/28/checking-types-in-javascript/

P.S.: Changes in line 37 "... cached === null || cached === undefined ..." is also an alternative solution of #162
( #162 )
@pygy
Copy link
Member

pygy commented Aug 7, 2014

The thing is... instanceof has its own issues.

In the first case, we could check if args.length > 1.

Beside that check, the only change required is this line.

data can't be problematic thanks to this line, and the case where cached is null or undefined is covered by your check.

The third case benign.

oleg8sh added a commit to oleg8sh/mithril.js that referenced this pull request Aug 8, 2014
@oleg8sh oleg8sh mentioned this pull request Aug 8, 2014
@oleg8sh
Copy link
Contributor Author

oleg8sh commented Aug 8, 2014

@pygy, You're absolutely right about instanceof.
This is named a "Cross-window Issues of instanceof" in the article from the first link.
And now I see @lhorie cleans out them from the next branch.

But {}.toString() has same issue for popups in IE, as mentioned in that article also.
So there is no easy way to satisfy any case.

PS: small useful change from this patch is extracted to #189.

@oleg8sh oleg8sh closed this Aug 8, 2014
@pygy
Copy link
Member

pygy commented May 22, 2015

On this topic, I'm currently pushing for the inclusion of a correct Object.prototype.toString in ES5Shim and polyfill.io so that we can rely completely on it.

es-shims/es5-shim#304
polyfillpolyfill/polyfill-service#401

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.

2 participants