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.

Sign up for GitHub

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

Jump to bottom

Ignore styles which no longer apply #804

Closed
wants to merge 5 commits into from
Closed

Ignore styles which no longer apply #804

wants to merge 5 commits into from

Conversation

fredden
Copy link
Member

@fredden fredden commented Mar 2, 2022 •
edited by m2-assistant bot
Loading

Description

When PageBuilder initialises, it moves styles from a <style> tag into data-{viewport}-styles attributes. If there are CSS selectors that do not match any elements, an error is thrown:

Screenshot_2022-03-02_11-17-34

This pull request fixes this bug.

Manual testing scenarios

  1. Use Magento import tool to set a product description (or other Page Builder-enabled attribute) which contains HTML, including some CSS that does not match any elements. (eg, <span>Sample text</span><style>#not-found { color: red; }</style>)
  2. Navigate to this product in the admin.
  3. Witness that Page Builder fails to initialise for this field, and saving the product is impossible (due to a different JavaScript error which stems from the same root cause).
  4. Apply the changes in this pull request.
  5. Witness that Page Builder initialises as expected, and it is possible to save the product.

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] Ignore styles which no longer apply #863: Ignore styles which no longer apply

@sdzhepa sdzhepa added the Priority: P1 Priority: P1 label May 5, 2022
@paras89
Copy link
Contributor

paras89 commented Jun 20, 2022

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@paras89
Copy link
Contributor

paras89 commented Jun 21, 2022

@fredden - can you pull develop branch into your fork's branch and fix a failing static test? After that we should be good to merge this PR!

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@paras89
Copy link
Contributor

paras89 commented Jun 27, 2022

@magento run Functional Tests B2B

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@paras89
Copy link
Contributor

paras89 commented Jul 28, 2022

@magento run Functional Tests B2B

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Hotel engcom-Hotel self-requested a review February 13, 2024 13:34
@engcom-Hotel
Copy link
Collaborator

@magento run all tests

@engcom-Hotel
Copy link
Collaborator

@magento create issue

@m2-assistant m2-assistant bot mentioned this pull request Feb 13, 2024
Open
5 tasks
@engcom-Hotel engcom-Hotel added the Project: Community Picked PRs upvoted by the community label Jul 17, 2024
@engcom-Hotel
Copy link
Collaborator

@magento run all tests

engcom-Hotel
engcom-Hotel approved these changes Jul 24, 2024
View reviewed changes
Copy link
Collaborator

@engcom-Hotel engcom-Hotel left a comment

Choose a reason for hiding this comment

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

Hello @fredden,

Thanks for the contribution!

The changes look good to me, hence approving the PR.

Thanks

@engcom-Bravo
Copy link
Collaborator

Hi @fredden,

Thanks for the collaboration & contribution!.

We have verified the issue in Latest 2.4-develop instance and the issue is not reproducible.Kindly refer the attached video.

Manual testing scenarios

  • Use Magento import tool to set a product description (or other Page Builder-enabled attribute) which contains HTML, including some CSS that does not match any elements. (eg, Sample text<style>#not-found { color: red; }</style>)
  • Navigate to this product in the admin.
  • Witness that Page Builder fails to initialise for this field, and saving the product is impossible (due to a different JavaScript error which stems from the same root cause).
Screen.Recording.2024-07-25.at.12.36.41.mp4

After importing the product with the description we are able to save the product successfully.

Could you please try to reproduce the issue in Latest 2.4-develop instance and could you please let us know if we are still missing anything.

Thanks.

@engcom-Bravo
Copy link
Collaborator

Hi @fredden,

Thank you for your contribution!

As mentioned here #804 (comment), please have a look and let us know if we are missing anything.

Till then closing this PR. Please feel free to reopen it as you want to update further.

Thank you!

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

@engcom-Hotel engcom-Hotel engcom-Hotel approved these changes

Assignees

@engcom-Hotel engcom-Hotel

Labels
Partner: Fisheye partners-contribution Priority: P1 Priority: P1 Project: Community Picked PRs upvoted by the community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Issue] Ignore styles which no longer apply
6 participants
@fredden @paras89 @engcom-Hotel @engcom-Bravo @sdzhepa @m2-github-services