Skip to content

Disable javascript minifying of TinyMCE 7 library, to fix a bug with…#39253

Closed
hostep wants to merge 1 commit intomagento:2.4-developfrom
hostep:fixes-tinymce7-issue-with-minifying-js
Closed

Disable javascript minifying of TinyMCE 7 library, to fix a bug with…#39253
hostep wants to merge 1 commit intomagento:2.4-developfrom
hostep:fixes-tinymce7-issue-with-minifying-js

Conversation

@hostep
Copy link
Copy Markdown
Contributor

@hostep hostep commented Oct 10, 2024

… loading language files from it.

Description (*)

I've seen it happening on Magento 2.4.7-p3 where TinyMCE v5 was updated to v7, but it also happens on 2.4-develop branch.

It's a minor bug, nothing really important I think, just making sure people can read the help section of the TinyMCE editor while using it.

So, when Magento has JS minification enabled, during SCD* it will transform any .js file it finds that doesn't already end its filename with .min.js to a minified version of that and rename the file from xxx.js to xxx.min.js.
However, in case of the TinyMCE 7 library, when it tries to load its language files from here, it doesn't expect them to have the filename of {language-code}.min.js and fails to load them.

In this PR, we prevent this from happening and disallow minification of js files whenever the pattern /tiny_mce_7/ is found in a path when the SCD* process processes js files.

*SCD = static content deploy

Related Pull Requests

Fixed Issues (if relevant)

None at the moment

Manual testing scenarios (*)

  1. Setup clean Magento shop (I'm using one without pagebuilder as its easier to showcase the bug, but you can trigger part of the bug also with pagebuilder enabled)
  2. Enable JS minification: bin/magento config:set dev/js/minify_files 1
  3. Remove all generated frontend files (if any) and flush caches, easiest is just to run bin/magento setup:upgrade
  4. Now switch to production mode, as JS minification only works in production mode: bin/magento deploy:mode:set production
  5. Go in the backoffice
  6. Create a new cms page
  7. Open your browsers inspector
  8. Click open the 'Content' tab
  9. Click inside the wysiwyg editor and try to launch the help with the keyboard shortcut mentioned at the bottom of the editor: alt/option+0

Currently at step 8 you see these error messages:

GET .../adminhtml/Magento/backend/en_US/tiny_mce_7/plugins/help/js/i18n/keynav/en.js net::ERR_ABORTED 404 (Not Found)
Refused to execute script from '.../adminhtml/Magento/backend/en_US/tiny_mce_7/plugins/help/js/i18n/keynav/en.js' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.
Uncaught (in promise) Script at URL ".../adminhtml/Magento/backend/en_US/tiny_mce_7/plugins/help/js/i18n/keynav/en.js" failed to load

And at step 9 the help doesn't open and you get another error message:

Uncaught (in promise) Script at URL ".../adminhtml/Magento/backend/en_US/tiny_mce_7/plugins/help/js/i18n/keynav/en.js" failed to load

With the fix from this PR, that's all solved.

Questions or comments

Adding automated tests for this will be hard, so I'm not doing it, hopefully that's okay, if not, please write them yourselves, I won't have the time to put into this task.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Disable javascript minifying of TinyMCE 7 library, to fix a bug with… #39263: Disable javascript minifying of TinyMCE 7 library, to fix a bug with…

@m2-assistant
Copy link
Copy Markdown

m2-assistant bot commented Oct 10, 2024

Hi @hostep. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@hostep
Copy link
Copy Markdown
Contributor Author

hostep commented Oct 10, 2024

@magento run all tests (is probably not needed for this PR, but lets just run them once)

@magento-automated-testing
Copy link
Copy Markdown

Failed to run the builds. Please try to re-run them later.

@hostep
Copy link
Copy Markdown
Contributor Author

hostep commented Oct 10, 2024

@magento run all tests

@engcom-Hotel
Copy link
Copy Markdown
Contributor

@magento create issue

@engcom-Hotel engcom-Hotel added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Oct 15, 2024
@Priyakshic Priyakshic added the Project: Community Picked PRs upvoted by the community label Nov 13, 2024
@engcom-Bravo
Copy link
Copy Markdown
Contributor

Hi @hostep,

Thanks for your Contribution!!.

As per this comment #39263 (comment) moving this PR to On Hold.

Thanks.

@hostep
Copy link
Copy Markdown
Contributor Author

hostep commented Dec 30, 2024

Closing, was apparantly fixed internally by:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Project: Community Picked PRs upvoted by the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue] Disable javascript minifying of TinyMCE 7 library, to fix a bug with…

5 participants