Polymer / polymer

Our original Web Component library.
https://polymer-library.polymer-project.org/
BSD 3-Clause "New" or "Revised" License
22.03k stars 2.02k forks source link

Array mutation methods cause IE to crash #2292

Closed kgastelum closed 2 years ago

kgastelum commented 9 years ago

Working on a component that data binds to an array of complex objects. The initial rendering is fine; however, when the data changes, using the mutation methods to modify the property will cause IE to crash.

    this.splice("shapedData.groupedData", 0, this.shapedData.groupedData.length);

for (var x: number = 0; x < newdata.length; ++x) { this.push("shapedData.groupedData", newdata[x]); }

dfreedm commented 9 years ago

Hmm, I can't seem to reproduce this in IE 10, 11, or Edge.

Can you try with https://github.com/Polymer/polymer/pull/2314 applied?

kgastelum commented 9 years ago

Thank you for the quick response…

I do not believe this is a polymer issue; rather, it is IE (go figure…). I have <div style=’display:table’… in my template and children of that div that are

etc… when changing the data using the array mutation methods, which causes the elements to be removed and rendered again, IE will crash.

I will try with #2314 just to see.

Thanks

From: Daniel Freedman [mailto:notifications@github.com] Sent: Wednesday, August 19, 2015 2:31 PM To: Polymer/polymer polymer@noreply.github.com Cc: Kurt Gastelum kurt.gastelum@elynxtech.com Subject: Re: [polymer] Array mutation methods cause IE to crash (#2292)

Hmm, I can't seem to reproduce this in IE 10, 11, or Edge.

Can you try with #2314https://github.com/Polymer/polymer/pull/2314 applied?

— Reply to this email directly or view it on GitHubhttps://github.com/Polymer/polymer/issues/2292#issuecomment-132752390.

lrbridge commented 8 years ago

I'm seeing the same behavior with array mutation methods and IE 11. I tried Polymer 1.2.1 but I still have the same issue. Looks like it crashes in polymer-mini.html on nativeAppendChild.call(container, node);.

Did you ever get it working @kgastelum ?

lrbridge commented 8 years ago

Posting this in case it helps someone else...

We used dom-repeat with fake table classes (instead of table/tr/td because of https://github.com/Polymer/polymer/issues/1567), but then it was crashing in IE unless we remove the display: table and display: table-row styles.

I finally figured out it works with 0.7.16 webcomponents.js but NOT with 0.7.16 webcomponents-lite.js, so we'll be using webcomponents.js now.

https://jsfiddle.net/fkgzceaa/3/

dfreedm commented 8 years ago

Wow that's nuts. The difference you're seeing is probably due to webcomponents.js using the ShadowDOM Polyfill (-lite never uses the polyfill), and that is probably smoothing over the IE bug internally.

mdwragg commented 8 years ago

+1 @robdodson @tjsavage - please can we we get help either working around this or getting a fix prioritised? This is affecting the browser support of GE Digital's Predix polymer-based Platform. We're getting crashes across IE10 and 11 with custom Polymer components that we've created. We're actively trying to work around it on our end but really need your help to get past this blocker.

As always, thanks.

(p.s. I could've sworn I sent this message yesterday but I can't find it - apologies if I've asked the same thing twice!).

tjsavage commented 8 years ago

We can prioritize, my understanding is this may likely take a good amount of digging to get a root cause & fix. @azakus any thoughts on a workaround?

mdwragg commented 8 years ago

Thanks @tjsavage! We really appreciate your help.

dfreedm commented 8 years ago

If you replace the table styling with display: block and display: inline-block, then this doesn't crash. This has to be some weird IE bug regarding table layout.

mdwragg commented 8 years ago

Yeah... We'd love to use a normal table but 'cause #1567 ;-)

dfreedm commented 8 years ago

I traced this into dom-repeat's _detachInstance function. If I try to step to this function, IE crashes: https://github.com/Polymer/polymer/blob/master/src/lib/template/dom-repeat.html#L623

@kevinpschaaf I might need some help here.

dfreedm commented 8 years ago
dfreedm commented 8 years ago

I can change this to container.appendChild(node), but that still crashes.

dfreedm commented 8 years ago

Ok, looks like removing the text node from its parent is the crasher.

dfreedm commented 8 years ago

WHOA, if you put strip-whitespace attribute on the dom-repeat elements, then it doesn't crash!

mdwragg commented 8 years ago

Whoa indeed... must try that! Thanks for digging in @azakus

mdwragg commented 8 years ago

Workaround works for me too. Thanks again for investigating @azakus!

kevin-wise commented 8 years ago

FWIW we ran across this today in a case where an array was being replaced with a new array which was a subset of the previous array. The same workaround works for us. We have dozens of places where we are using arrays with dom-repeat but only one seems to need the workaround. That's a little disturbing!

kevin-wise commented 8 years ago

Any chance of making strip-whitespace the default? If it fixes the issue and boosts performance, seems like it should be considered. I'm guessing most people use block-level elements inside templates so whitespace is rarely significant. In those cases there could always be the option to turn it off.

TimvdLippe commented 8 years ago

@kevin-wise-infusionsoft There is an already open issue to discuss this feature: #3363

ghost commented 8 years ago

So besides a strip-whitespace workaround is there any actual fix for this year old issue? I use splice a lot and need to support IE 11.

Still seeing the issue on Polymer 1.5.0.

DarkSideOfMo0n commented 7 years ago

Up! It seems in Polymer 1.9.1 this is still an Issue.

ntarora commented 6 years ago

Yep still an issue in 1.11.0, no issues applying the workaround

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!