PIE icon indicating copy to clipboard operation
PIE copied to clipboard

Fix for issue 149

Open gruber76 opened this issue 13 years ago • 9 comments

currentStyle is Async in IE, and/or may be null if an element is created using createElement. Either way, PIE needs to make sure currentStyle is present before accessing its methods.

(see http://stackoverflow.com/questions/3722736/weird-ie-behavior-currentstyle-returns-null)

gruber76 avatar Mar 19 '12 03:03 gruber76

Thanks for the patch. I'm not sure this would work though -- currentStyle.getAttribute is a function so I'd expect it to throw a "getAttribute is not a function" error if it's set to a plain object.

Do you have a testcase you've been able to test this fix against? That's been the only issue keeping me from fixing this myself, I don't have a way to reproduce the issue so I don't know if a fix works or not.

lojjic avatar Mar 19 '12 03:03 lojjic

Yeah, that was a hack of a patch, I realized about ten seconds after I hit "request." Sorry. I'll get a test for you tomorrow, as I have it breaking 100% on a pie.js loop.

gruber76 avatar Mar 19 '12 03:03 gruber76

Awesome, thanks. :)

Since we're discussing it I do have some additional thoughts about this issue...

  1. Since PIE really can't do much of anything without currentStyle being present, I'd prefer to short-circuit as soon as possible (and only once), for instance perhaps in Element#update().

  2. If we're preventing PIE from completing an update, then we need to make sure that we don't leave it hanging like that and have elements remain unstyled. If the process of IE's async currentStyle initialization doesn't itself trigger PIE to re-update once currentStyle becomes available, then we need to queue something up to try again after a delay.

lojjic avatar Mar 19 '12 03:03 lojjic

@gruber76 Any word on that testcase? I'd like to get a proper fix in asap.

lojjic avatar Mar 25 '12 16:03 lojjic

FYI here's my potential fix -- https://github.com/lojjic/PIE/tree/currentStyle-check

This checks for the presence of currentStyle at the start of each update, and if it's not present then it exits right away and sets up a timer to attempt the update again after a delay.

This is, of course, untested since I don't yet have a testcase. ;)

lojjic avatar Mar 25 '12 17:03 lojjic

Ugh, sorry I got this error in an Angular project... it may be that the script is trying to run .attach before the element is actually IN the DOM, but after spending longer trying to pare it down than I really had time for I had to get back to paid work. Here's a snippet that creates the PIE error in IE9: https://gist.github.com/2202107

gruber76 avatar Mar 26 '12 01:03 gruber76

See my comment on your gist, there seems to be an error in the gist code and fixing that allows IE9 to render without error.

Unfortunately without a testcase I'll have to close this one as not reproducible. :/

lojjic avatar Mar 26 '12 02:03 lojjic

You can reproduce this by setting -pie-poll to true for an element and remove the element frmo the DOM tree by calling innerHTML = "" in the parent or.

add another class to the element, using .className += " newClass" and the new class has a different background color

https://github.com/lojjic/PIE/issues/210

balu980 avatar Apr 17 '12 20:04 balu980

@balu980 I tried to create a testcase that does the steps you described, but was unable to trigger an error.

Are you able to create a testcase, or point me to a URL that triggers it?

lojjic avatar Apr 29 '12 03:04 lojjic