magento / magento2-page-builder

Magento2 PageBuilder
Other
78 stars 58 forks source link

unlockImageSize method leaves unintended styles behind #805

Open braders opened 2 years ago

braders commented 2 years ago

Preconditions (*)

  1. Magento 2.4.3-p1 on Adobe Commerce Cloud
  2. Reproducable in all environments (development, staging, prod)

Steps to reproduce (*)

  1. Create a new product (or page, category etc)
  2. Add a text element to the page builder
  3. Using the WYSIWYG insert an image from the media library. Tick the “Constraign dimensions“ tickbox a couple of times (this ensures tinymce sets with/height attributes, which only happens once the image src has finished loading).
  4. Observe through the TinyMCE "source code" functionality that the image now has width and height attributes, but no style tag.
  5. Save the product/page/category.
  6. Re-open the WYSIWYG and look at the source using TinyMCE "source code" functionality again. Observe that style="width: ...px; height: ...px;" has now been added.

Expected result (*)

  1. The inline styles style="width: ...px; height: ...px;" should not have been added to the images.

Actual result (*)

  1. Inline syles are added to images elements. This is causing problems with the display of the images in a responsive context.

Technical comments

The problematic code is the below (module-page-builder/view/adminhtml/web/js/utils/editor.js):

/**
   * Lock all image sizes before initializing TinyMCE to avoid content jumps
   *
   * @param element
   */

  function lockImageSize(element) {
    [].slice.call(element.querySelectorAll("img")).forEach(function (image) {
      if (image.style.width.length === 0) {
        image.style.width = /^\d+$/.test(image.getAttribute("width")) ? image.getAttribute("width") + "px" : image.getAttribute("width");
        image.setAttribute("data-width-locked", "true");
      }

      if (image.style.height.length === 0) {
        image.style.height = /^\d+$/.test(image.getAttribute("height")) ? image.getAttribute("height") + "px" : image.getAttribute("height");
        image.setAttribute("data-height-locked", "true");
      }
    });
  }
  /**
   * Reverse forced image size after TinyMCE is finished initializing
   *
   * @param element
   */

  function unlockImageSize(element) {
    [].slice.call(element.querySelectorAll("img")).forEach(function (image) {
      if (image.getAttribute("data-width-locked")) {
        image.style.width = null;
        image.removeAttribute("data-width-locked");
      }

      if (image.getAttribute("data-height-locked")) {
        image.style.height = null;
        image.removeAttribute("data-height-locked");
      }
    });
  }

In unlockImageSize pagebuilder tries to remove the style attibutes with image.style.width = null; and image.style.height = null;.

However, inspecting the img element in devtools within the tinymce content editor, there is also data-mce-style="width: 1000px; height: 400px;". It appears TinyMCE is converting this into a style attribute when the editor content is serialised before save.

I am assuming there could be other legitimate styles within data-mce-style, so I'm not sure it's safe to just remove the data-mce-style attribute - some sort of regex might be needed to remove just the width and height components.

m2-assistant[bot] commented 2 years ago

Hi @braders. Thank you for your report. To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this