Remove more Internet Explorer code

[justinbeaty]

Description (*)

Despite #1073, MSIE somehow lives on in our codebase. I am attempting to remove more instances of Internet Explorer.

This is a breaking change because of the removal of javascript functions toggleSelectsUnderBlock and setLoaderPosition. Also because #1073 was into 20.0 as well. I instead made those functions stubs, so there shouldn't be a breaking change here.

This PR is not tested yet! I should test to see what happens if a user has duplicated a template that still calls toggleSelectsUnderBlock. I am not sure if it will break the JS or just log an error. If it breaks things, I can just make the content of those functions empty. See above

Also I removed js/lib/flex.js. I think it remained from #2271. Now in separate PR #2434

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes the issue of having to see Internet Explorer compatibility code ;)

Manual testing scenarios (*)

1. Test various parts of the code and make sure things don't break.

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[lgtm-com]

This pull request fixes 2 alerts when merging 721e8e8 into 71998f5 - view on LGTM.com

fixed alerts:

• 1 for Call to eval-like DOM function
• 1 for Missing variable declaration

[kiatng]

How about this?

magento-lts/app/design/adminhtml/default/default/template/media/editor.phtml

Line 21 in e6f0851

<?php echo $this->helper('adminhtml/js')->includeScript('lib/flex.js') ?>

[justinbeaty]

@kiatng I’ll delete that and the script block too.

Edit: I should also make sure it’s clear I haven’t tested any of this. I just noticed some things during #2426 and wanted to commit them. I plan to test later and see what I broke. :)

[ADDISON74]

The content of the /js/lib/flex.js file checks for Adobe Player version. The new file uploader was introduced in 1.9.3 and it has nothing to do with flex.js. I think this file is orphaned in the repository but its removal must be done after a detailed analysis. From what I can see we have files in the directory /lib/flex/.

All references to Internet Explorer must be removed from OpenMage. The last version of IE11 is no longer supported from June 15, 2022. As in the case of Adobe Flash Player, Microsoft has decided to remove it from Windows 10 in the following upgrades. It was the best browser to download other browser like Google Chrome.

[justinbeaty]

The content of the /js/lib/flex.js file checks for Adobe Player version. The new file uploader was introduced in 1.9.3 and it has nothing to do with flex.js. I think this file is orphaned in the repository but its removal must be done after a detailed analysis. From what I can see we have files in the directory /lib/flex/.

/lib/flex/ is removed from 20.0 already, but not /js/lib/flex.js. However I think I will move this portion to another PR so it can be separately tested.

[ADDISON74]

You are doing very well. We should establish as a rule so that a PR does not contain too many changes, especially on different aspects. It is easier to approve a PR that deals with an issue and is tested and approved quickly, than to keep testing it and delay approval because there are too many issues solved in it.

[ADDISON74]

Cleaning the OM code of IE chunks is much more extensive than we thought. I searched in the project for IE 6, IE 7, ..., IE 11 and I found references even in the layout files in /app/designs (e.g. "if IE lower than 6 load this css/js file". The hardest task is in the files where there are no comments related to IE. Relics left in the code that I think are over 10 years old. Today someone told me that what he learned 5 years ago in his field is no longer fully valid and that things have changed and he had to update himself with the latest news. That's what we do every day here

[justinbeaty]

@ADDISON74 make sure you're only searching the 20.0 branch. IE compatibility was not removed from 1.9.4.x and many of the references you mention sound like things removed in #1073

[ADDISON74]

I know about #1073, I have a few comments there.

I didn't check if it was completely removed from 20.0. This time it is a special case, it is about an old browser which is no longer maintained, which has security and data presentation issues in the Frontend. IE 8 doesn't understand secured connection. OM should no longer support this browser. One is to ensure the compatibility of some extensions and another is to encourage the use of an expired and dangerous browser. Maybe we still find IE9 - IE11 installed through Windows 7 and 10, but I already see statistics where IE doesn't even exist anymore. In addition, Microsoft has decided that in the next phase of Windows it will eliminate IE once and for all. , It should have been removed from 1.9.4.x as well.

[justinbeaty]

It should have been removed from 1.9.4.x as well.

It was not removed from 1.9.4.x because it was a breaking change. But also major changes like that shouldn't happen to an LTS release... no need to.

If there are still MSIE code in 20.0 I will remove it as I come across it.

[lgtm-com]

This pull request fixes 1 alert when merging 024a8d5 into 71998f5 - view on LGTM.com

fixed alerts:

• 1 for Missing variable declaration

[ADDISON74]

I would remove from OM all the code related to Internet Explorer. This BC that we keep citing in posts when it comes to OM-19 I find it useful in the case of extensions/modules, a specific server configuration, but not in the case of an expired browser. Basically we want to protect the interests of a visitor of an OM-based store who refuses to update to a modern browser. IE 6 - 11 have big security issues, some versions don't even establish TLS1.2 connection anymore. Removing the OM code will not create trouble, only for those who use the expired browser. In conclusion, I don't see a BC issue at all in this case.

[justinbeaty]

We have to double check why #1073 was BC breaking. I don't think it was just because we remove MSIE compatibility, but I think it was a function declaration that changes. This could be an actual BC break for any module that might have extended that function.

[justinbeaty]

FYI this is on hold until #2426 / #2475 is merged into 20.0 just to reduce conflicts.

[fballiano]

FYI this is on hold until #2426 / #2475 is merged into 20.0 just to reduce conflicts.

Both of them are merged now😊

[justinbeaty]

FYI this is on hold until #2426 / #2475 is merged into 20.0 just to reduce conflicts.

Both of them are merged now😊

👍 Thanks! Updated and pushed. I am testing now.

[lgtm-com]

This pull request fixes 1 alert when merging c5db926 into d448eeb - view on LGTM.com

fixed alerts:

• 1 for Missing variable declaration

[lgtm-com]

This pull request fixes 1 alert when merging 152dae9 into bc4b2cd - view on LGTM.com

fixed alerts:

• 1 for Missing variable declaration