Skip to content

Conversation

@jonathanKingston
Copy link
Contributor

Description

Inline <script tags are a source of code injection, the browser can protect against this with CSP to block inline scripts. However Magento has a lot of inline scripts that need removing bit-by-bit.

Fixed Issues (if relevant)

  1. Removal of an inline script block

Manual testing scenarios

  1. Ensure browsers with no localstorage still support storage

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)
  • All automated tests passed successfully (all builds on Travis CI are green)

@orlangur orlangur requested a review from VladimirZaets March 24, 2018 13:26
@maghamed maghamed added the Event: distributed-cd Distributed Contribution Day label Mar 24, 2018
@jonathanKingston jonathanKingston force-pushed the 2.3-develop_removal-of-pollyfill-inline branch from fb92257 to 3668651 Compare March 25, 2018 19:22
@jonathanKingston
Copy link
Contributor Author

Updating to fix the coding style since it was moved...

@jonathanKingston jonathanKingston force-pushed the 2.3-develop_removal-of-pollyfill-inline branch from 3668651 to ab934f6 Compare March 26, 2018 09:51
@jonathanKingston
Copy link
Contributor Author

Updated code style which is now passing locally.

@jonathanKingston jonathanKingston force-pushed the 2.3-develop_removal-of-pollyfill-inline branch 2 times, most recently from bb1bbb2 to 495818f Compare March 27, 2018 08:50
@jonathanKingston jonathanKingston force-pushed the 2.3-develop_removal-of-pollyfill-inline branch from 495818f to 823f07a Compare March 28, 2018 11:17
@VladimirZaets VladimirZaets self-assigned this Apr 2, 2018
Copy link
Contributor

@VladimirZaets VladimirZaets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jonathanKingston. Please fix it using the backward compatible way. We can't delete block from the layout. Please look at our document for more details about backward compatibility. You can leave the block declaration, and use <script type="text/x-magento-init"> to initialize js file.

@jonathanKingston
Copy link
Contributor Author

@VladimirZaets I'm a little confused by your comment, this should be fully backwards compatible no? If I use an init block it will be asynchronously loaded where as this will load synchronously in the same manner as the embedded block currently.
Are you suggesting that the app/code/Magento/Theme/view/frontend/templates/js/polyfill.phtml file should still exist? If so the only way would be for this to import the new JS file I have created.

@VladimirZaets
Copy link
Contributor

@jonathanKingston We had discussion about this case and take desition that your fix is ok, so we will merge your fix in the near time, thanks.

@magento-engcom-team
Copy link
Contributor

Hi @jonathanKingston. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.3.0 release.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants