Add NodeList.forEach polyfill to support IE11

Hello, could you add more details as to where we would need that? If we make it available we need to maintain it, etc. Supporting IE11 doesn't mean polyfilling IE11 to behave like the other browsers.

I don't think this is how we should manage polyfills. We are using babel so I would expect to use something based on that to make it easier to maintain and keep up to date.

Fixing target version.

At least it should be done like array.find and object.assign polyfill.

Hi nod,

This needs to be added for MediaLibraryWidgetSortable file: core/modules/media_library/js/media_library.widget.js
The issue on IE11 that I have is that when you have content type with a media field using the media library and you try to upload a image and save is not doing anything as the javascript is breaking, on the console you will get the following error:

Object doesn't support property or method 'forEach'

Then I debug and I found that on IE11 this line is returning:
var selection = context.querySelectorAll('.js-media-library-selection');

Then it means Nodelist is not supporting forEach

Thanks,

There is also a forEach in drupal.js on or around line 22 (for 8.9.x):
Object.keys(behaviors || {}).forEach(function (i) {

I think rather than trying to avoid using modern JS methods like forEach we should add the polyfill to support IE11 and call it good enough. Yes IE11 will be here for the next few years which is extremely annoying, but why refactor core code to accommodate it? If we don't add the polyfill to core, people will probably add to themes anyway.

Patch in #10 works for me so I am going to mark this as RTBC, but please adjust if we feel like this is not the right approach.

Thank you for this issue and for the collaboration!

Yes, but the file is not in the right place, it should be in core/misc/polyfills/ and have a library defined like we have for drupal.array.find in core.libraries.yml and not just added indiscriminately to the drupal library.

Also it would be good to have at least one sentence in the issue summary to explain why we need it and where we want to use it. Adding it just because will probably not be accepted by committers.

Got it. That makes total sense. Here is a patch that adds the polyfill/library in the desired location per the comment. I am going to test this first and then if it works I will mark as "Needs Review."

Well #18 didn't work. Let's try this one.

Again!

FYI the patch should be against 9.1.x :)

Woooo let the patches keep rolling in! Here is one against 9.1.x.

Patch in #22 applies to both 9.1.x and 8.9.x and test looks good! Let me know what you think. Thanks and hope this works/helps folks!

Can you elaborate in the issue summary where in core we're using forEach.

Any reason we can't just wrap it in Object.values?

Added info about forEach in drupal.js to issue summary.

The issue is described in #13, updated the issue summary.

libraries are ordered alphabetically in libraries file.
The comments were not up to date in the polyfill file
it's a polyfill of the nodelist object, not the array object.

Should be good to go.

Any reason we can't just wrap it in Object.values?

Unfortunately, IE11 does not support Object.values

I'm setting this to "Needs Work" to help make this visible, but the patch itself looks good. The implementation matches the pattern established by similar polyfills, and the polyfill itself is well established.

There's just a few housekeeping bits that I'd like to have in place before getting this in front of a committer:

• This should get a change record similar to this recently-added change record for the Array.find polyfill
• Tagging with "Needs followup" as there ought to be an issue to clean up the many uses of Array.prototype.forEach.call() that can be simplified to just .forEach() once this polyfill is available.

Created the change record and followup: #3176386: [PP-1] Replace Array.prototype.forEach.call usages that aren't needed now that a polyfill exists. Those were the only things blocking me from RTBCing it, so now I can.

Hi,

This is an embarrassing question but then again I'm very new to Drupal. I know how to apply patches on a module but not sure of the same about Drupal core.

I'm getting "Object doesn't support property or method 'forEach'" error in drupal.js on IE 11 so I assume the patch for 8.9.x would solve the problem. However I don't know how to apply the patch to core.

Could anyone please guide on how I can apply the patch? I'm on Drupal 8.9.7.

Thanks!!

I meant to RTBC in #28. I was good with the patch, it just needed the CR and followup, which I took care of.

@devKhairul Does this documentation page help at all? If you are using composer and want to apply patches for core, the project is drupal/core.

Committed 2d1b0a4 and pushed to 9.1.x. Thanks!

@lauriii I found a similar document and it worked for me. Thanks for sharing!!