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

fallback to elem.style for disconnected nodes, fixes #10254 and #8388 #553

Closed
wants to merge 4 commits into from

Conversation

mikesherov
Copy link
Member

disconnected nodes don't have currentStyle in IE<9!

@@ -290,6 +290,10 @@ if ( document.documentElement.currentStyle ) {
rsLeft = elem.runtimeStyle && elem.runtimeStyle[ name ],
style = elem.style;

if ( !ret && style ) {
Copy link
Member

Choose a reason for hiding this comment

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

it's ok if ret is 0 here right? maybe this should be ret === undefined

Copy link
Member Author

Choose a reason for hiding this comment

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

hmmm... maybe. I'm not sure it's undef... we may also want to be checking empty strings and null as well. checking for 0 is a perf. optimization... so I'm not 100% sure it'll work in all cases.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should check for null and I really don't think we need to check for 0. Getting a null value would only occur when the specific style does not exist on currentStyle, in which case it should return null. And 0 can be a valid computed value that should not be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, so just check for undef it is... I'll make sure the unit test passes here. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@timmywil, i just tested this. IE8 returns null in my unit tests... i need to check for null/undefined for disconnected nodes! i can get rid of the 0 check though, i think.

Copy link
Member

Choose a reason for hiding this comment

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

It's returning null for a non-existent property? I guess the property exists, but IE chooses to return null on disconnected nodes?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then just a check for null should be sufficient. When you said "don't have currentStyle", I assumed the property was non-existent.

Copy link
Member Author

Choose a reason for hiding this comment

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

it returns null for the currentStyle property, and the right value in elem.style for background-image and top.

@timmywil
Copy link
Member

Landed in commit e502012.

@timmywil timmywil closed this Oct 22, 2011
mescoda pushed a commit to mescoda/jquery that referenced this pull request Nov 4, 2014
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants