Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape product names and translations inside js #2673

Merged
merged 2 commits into from
May 29, 2023

Conversation

Judx
Copy link
Contributor

@Judx Judx commented Nov 2, 2022

Description (*)

Product names and translations are not escaped before being echoed into a JavaScript block.
If the product name and/or translation would include a double quote (") that would break the JavaScript.

Related Pull Requests

#2366

Manual testing scenarios (*)

i.e. JavaScript breaks when MAP with TIER-prices enabled
  1. Create a new product with a a double quote " in the name
  2. Enable MAP and "Show On Gesture"
  3. Add MSRP, TIER and GROUP prices for the product
  4. Go to the product page and try to "Click for price" -> Does not work as the JavaScript is broken
Catalog.Map.addHelpLink(
                    $('msrp-popup-1RFQl4noKSgLqDjmRse6K'),
                    "myproduct"",
...

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

@github-actions github-actions bot added Component: Bundle Relates to Mage_Bundle Component: Catalog Relates to Mage_Catalog Component: Checkout Relates to Mage_Checkout Component: Downloadable Relates to Mage_Downloadable Template : base Relates to base template Template : default Relates to base template Template : rwd Relates to rwd template labels Nov 2, 2022
@elidrissidev
Copy link
Member

LGTM. Can you please setup your git credentials and amend the commits.

@Judx Judx force-pushed the escape-js branch 2 times, most recently from a18f8c6 to 170cd7d Compare November 3, 2022 10:40
@Judx
Copy link
Contributor Author

Judx commented Nov 3, 2022

I signed the commits and they are now verified with my key.

elidrissidev
elidrissidev previously approved these changes Nov 3, 2022
@sreichel
Copy link
Contributor

sreichel commented Nov 6, 2022

LGTM, but i can't reproduce. I have no JS error in console.

@sreichel sreichel added review needed Problem should be verified and removed needs investigation labels Jan 4, 2023
@fballiano fballiano changed the base branch from 1.9.4.x to main May 13, 2023 13:06
@github-actions github-actions bot removed Template : default Relates to base template Component: Downloadable Relates to Mage_Downloadable labels May 13, 2023
@fballiano
Copy link
Contributor

I've rebased it and fixed conflicts, it worked fine :-)

@fballiano
Copy link
Contributor

the force push for the rebase dismissed @elidrissidev review sadly

elidrissidev
elidrissidev previously approved these changes May 29, 2023
@fballiano fballiano merged commit 8ef41f1 into OpenMage:main May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Bundle Relates to Mage_Bundle Component: Catalog Relates to Mage_Catalog Component: Checkout Relates to Mage_Checkout review needed Problem should be verified Template : base Relates to base template Template : rwd Relates to rwd template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants