Fix #866: Add element existence check to prevent Page Builder editor crashes

artttj commented 2 months ago

Description

We encountered a scenario where missing HTML elements in Page Builder led to runtime errors due to direct calls to setAttribute.

To address this, we added an existence check before setting attributes. This change prevents the editor from breaking and improves stability by handling cases where elements are missing.

Ideally, the core should include such checks to enhance overall resilience and user experience.

Story

N/A

Bug

N/A

Task

N/A

Fixed Issues

magento/magento2-page-builder#866: Graceful Handling of Null element in Page Builder to Prevent Editor Failure

Builds

N/A

Related Pull Requests

N/A

Manual Testing Scenarios

Please test the following scenarios to verify the changes:

Remove data-pb-style Attribute:
- Remove the data-pb-style attribute from an element with applied styles.
- Ensure the Page Builder continues to work without issues.
Modify data-pb-style Hash:
- Change the data-pb-style attribute's value to a non-relevant hash.
- Confirm that the Page Builder handles this gracefully and remains functional.
Remove HTML Element:
- Delete the HTML element to which styles are applied.
- Check that the Page Builder does not crash and continues to operate normally.

Note: This issue was observed when using the DeepL API, but similar problems might occur in other scenarios. The preview editor should remain stable.

Questions or comments

N/A

Checklist

[x] Pull request has a meaningful description of its purpose
[x] All commits are accompanied by meaningful commit messages
[ ] All new or changed code is covered with unit/integration tests (if applicable)
[x] 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)

artttj commented 2 months ago

@magento run all tests

engcom-Hotel commented 1 month ago

@magento run all tests

engcom-Hotel commented 1 month ago

After discussing it internally, it was found that the automated test is not required for this change. Hence moving it to ready for testing.

Thanks

engcom-Hotel commented 1 month ago

Hello @artttj,

Thanks for the contribution!

:x: QA not Passed

We have tried to reproduce the actual issue in the 2.4-develop branch with pagebuilder's develop branch, but the issue is not reproducible for us. Please refer to the below screencast for reference:

https://github.com/user-attachments/assets/38e9a8ff-1cca-42c4-9bba-6856b17995f1

Let us know if we missed anything.

Thanks

engcom-Hotel commented 2 weeks ago

Hello @artttj,

This PR is being closed since it has not been updated in a long time. Please feel free to reopen or raise a new PR.

Thanks

magento / magento2-page-builder