MC-5691: Implement better developer error reporting

- Handle invalid mark up present in field before load
- Remove deprecated usage of previewData

Author: davemacaulay

app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type/preview.js

@@ -36,8 +36,8 @@ export default function createContentType(
 ): Promise<ContentTypeInterface | ContentTypeCollectionInterface> {
     return new Promise(
         (resolve: (contentType: ContentType | ContentTypeCollection) => void,
-         reject: (error: string) => void
-    )=> {
+         reject: (error: string) => void,
+    ) => {
         loadModule([config.component], (contentTypeComponent: typeof ContentType | typeof ContentTypeCollection) => {
             try {
                 const contentType = new contentTypeComponent(

app/code/Magento/PageBuilder/view/adminhtml/web/js/master-format/read/configurable.js

@@ -4,9 +4,9 @@
  */
 
 import loadModule from "Magento_PageBuilder/js/utils/loader";
+import ContentTypeCollectionInterface from "../content-type-collection.d";
 import ContentTypeConfigInterface from "../content-type-config.d";
 import ContentTypeInterface from "../content-type.d";
-import ContentTypeCollectionInterface from "../content-type-collection.d";
 import converterResolver from "./converter-resolver";
 import Master from "./master";
 import MasterCollection from "./master-collection";
@@ -24,7 +24,7 @@ export default function create(
     config: ContentTypeConfigInterface,
 ): Promise<Master | MasterCollection> {
     return new Promise(
-        (resolve: (masterComponent: Master | MasterCollection) => void, reject: (error: string) => void
+        (resolve: (masterComponent: Master | MasterCollection) => void, reject: (error: string) => void,
     ) => {
         observableUpdaterFactory(config, converterResolver).then((observableUpdater) => {
             loadModule([config.master_component], (masterComponent: typeof Master | typeof MasterCollection) => {

app/code/Magento/PageBuilder/view/adminhtml/web/js/stage-builder.js

@@ -4,9 +4,9 @@
  */
 
 import loadModule from "Magento_PageBuilder/js/utils/loader";
+import ContentTypeCollectionInterface from "../content-type-collection.d";
 import ContentTypeConfigInterface from "../content-type-config.d";
 import ContentTypeInterface from "../content-type.d";
-import ContentTypeCollectionInterface from "../content-type-collection.d";
 import observableUpdaterFactory from "./observable-updater-factory";
 import Preview from "./preview";
 import PreviewCollection from "./preview-collection";
@@ -24,7 +24,7 @@ export default function create(
     config: ContentTypeConfigInterface,
 ): Promise<Preview | PreviewCollection> {
     return new Promise(
-        (resolve: (previewComponent: Preview | PreviewCollection) => void, reject: (e: string) => void
+        (resolve: (previewComponent: Preview | PreviewCollection) => void, reject: (e: string) => void,
     ) => {
         observableUpdaterFactory(config, previewConverterResolver).then((observableUpdater) => {
             loadModule([config.preview_component], (previewComponent: typeof Preview | typeof PreviewCollection) => {

app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type-factory.ts

@@ -93,8 +93,13 @@ export default class Preview {
      * @returns {string}
      */
     get previewTemplate(): string {
-        const appearance = this.previewData.appearance ? this.previewData.appearance() as string : undefined;
-        return appearanceConfig(this.config.name, appearance).preview_template;
+        const appearance = this.parent.dataStore.get("appearance") ?
+            this.parent.dataStore.get("appearance").toString() : undefined;
+
+        return appearanceConfig(
+            this.config.name,
+            appearance
+        ).preview_template;
     }
 
     /**

app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/master-factory.ts

@@ -14,6 +14,7 @@ import MassConverterPool from "../../mass-converter/converter-pool";
 import massConverterPoolFactory from "../../mass-converter/converter-pool-factory";
 import propertyReaderPoolFactory from "../../property/property-reader-pool-factory";
 import {ReadInterface} from "../read-interface";
+import Config from "../../config";
 
 /**
  * @api
@@ -27,7 +28,7 @@ export default class Configurable implements ReadInterface {
      * @returns {Promise<any>}
      */
     public read(element: HTMLElement): Promise<any> {
-        const role = element.getAttribute("data-role");
+        const role = element.getAttribute(Config.getConfig("dataRoleAttributeName"));
         const config = appearanceConfig(role, element.getAttribute("data-appearance"));
         const componentsPromise: Array<Promise<any>> = [
             propertyReaderPoolFactory(role),
@@ -40,13 +41,13 @@ export default class Configurable implements ReadInterface {
                 let data = {};
                 for (const elementName of Object.keys(config.elements)) {
                     const elementConfig = config.elements[elementName];
-                    const currentElement = element.getAttribute("data-element") === elementName
-                        ? element
-                        : element.querySelector<HTMLElement>("[data-element = '" + elementName + "']");
+                    const currentElement = this.findElementByName(element, elementName);
 
+                    // If we cannot locate the current element skip trying to read any attributes from it
                     if (currentElement === null || currentElement === undefined) {
                         continue;
                     }
+
                     if (elementConfig.style.length) {
                         data = this.readStyle(
                             elementConfig.style,
@@ -56,6 +57,7 @@ export default class Configurable implements ReadInterface {
                             converterPool,
                         );
                     }
+
                     if (elementConfig.attributes.length) {
                         data = this.readAttributes(
                             elementConfig.attributes,
@@ -65,16 +67,20 @@ export default class Configurable implements ReadInterface {
                             converterPool,
                         );
                     }
+
                     if (undefined !== elementConfig.html.var) {
                         data = this.readHtml(elementConfig, currentElement, data, converterPool);
                     }
+
                     if (undefined !== elementConfig.tag.var) {
                         data = this.readHtmlTag(elementConfig, currentElement, data);
                     }
+
                     if (undefined !== elementConfig.css.var) {
                         data = this.readCss(elementConfig, currentElement, data);
                     }
                 }
+
                 data = this.convertData(config, data, massConverterPool);
                 resolve(data);
             }).catch((error) => {
@@ -83,6 +89,25 @@ export default class Configurable implements ReadInterface {
         });
     }
 
+    /**
+     * Find the element for the current content type by it's name, avoiding searching in other content types by
+     * removing any other element which contains it's own data-role.
+     *
+     * @param {HTMLElement} element
+     * @param {string} name
+     * @returns {HTMLElement}
+     */
+    private findElementByName(element: HTMLElement, name: string): HTMLElement {
+        const currentElement = $(element).clone();
+        // Remove all child instances of data-role elements
+        currentElement.find(`[${Config.getConfig("dataRoleAttributeName")}]`).remove();
+
+        // Find the element in the modified close with a matching element name
+        return currentElement.attr("data-element") === name
+            ? currentElement[0]
+            : currentElement[0].querySelector<HTMLElement>(`[data-element=${name}]`);
+    }
+
     /**
      * Read attributes for element
      *

app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/preview-factory.ts

@@ -87,7 +87,7 @@ function createElementContentType(
     parent = parent || stage;
     const role = element.getAttribute(Config.getConfig("dataRoleAttributeName"));
     if (!role) {
-        return Promise.reject(`Invalid master format: Content type element does not contain 
+        return Promise.reject(`Invalid master format: Content type element does not contain
             ${Config.getConfig("dataRoleAttributeName")} attribute.`);
     }
 
@@ -120,8 +120,8 @@ function getElementData(element: HTMLElement, config: ContentTypeConfigInterface
         return "";
     });
 
-    return new Promise((resolve: (result: object) => void, reject: (e: string) => void) => {
-        const role = element.dataset.role;
+    return new Promise((resolve: (result: object) => void) => {
+        const role = element.getAttribute(Config.getConfig("dataRoleAttributeName"));
         if (!Config.getConfig("content_types").hasOwnProperty(role)) {
             resolve(result);
         } else {
@@ -147,15 +147,17 @@ function getElementData(element: HTMLElement, config: ContentTypeConfigInterface
  * @param {HTMLElement} element
  * @returns {Array<HTMLElement>}
  */
-function getElementChildren(element: HTMLElement): Array<HTMLElement> {
+function getElementChildren(element: HTMLElement): HTMLElement[] {
     if (element.hasChildNodes()) {
         let children: any[] = [];
 
         _.forEach(element.childNodes, (child: HTMLElement) => {
-            if (child.hasAttribute(Config.getConfig("dataRoleAttributeName"))) {
-                children.push(child);
-            } else {
-                children = getElementChildren(child);
+            if (child.nodeType === Node.ELEMENT_NODE) {
+                if (child.hasAttribute(Config.getConfig("dataRoleAttributeName"))) {
+                    children.push(child);
+                } else {
+                    children = getElementChildren(child);
+                }
             }
         });