Parsys Limit code prevents adding components in AEM 6.4.2.0 (SP2)

[ahmed-musallam]

NOTE: Reporting this issue here as an FYI, the fix for this should probably be done by adobe in a CFP.

Required Information

• [x] AEM Version, including Service Packs, Cumulative Fix Packs, etc: _AEM 6.4.2.0 (SP2)__
• [x] ACS AEM Commons Version: __3.19 - but i imagine it happens for earlier versions___
• [x] Reproducible on Latest? yes

Expected Behavior

1. Create a new Page that has a responsive grid
2. Add an Experience Fragment to the page - to any responsive grid (if multiple)
3. Try to add a component using the "+" editor action in the editor bar for the responsive grid
4. The component picker dialog should show

Actual Behavior

The following error is thrown and the component picker does not show:

An error has occured during execution of the selected action: code.indexOf is not a function -> TypeError: code.indexOf is not a function
    at Object.ns.util.sanitizeCQHandler (/libs/cq/gui/components/authoring/editors/clientlibs/sites/page.js:31462:62)
    at /libs/cq/gui/components/authoring/editors/clientlibs/core.js:7508:46
    at Array.forEach (<anonymous>)
    at cleanActions (/libs/cq/gui/components/authoring/editors/clientlibs/core.js:7491:21)
    at cleanEditableActions (/libs/cq/gui/components/authoring/editors/clientlibs/core.js:7476:38)
    at Object.ns.configParser (/libs/cq/gui/components/authoring/editors/clientlibs/core.js:7664:9)
    at new constructor (/libs/cq/gui/components/authoring/editors/clientlibs/core.js:8822:29)
    at HTMLUnknownElement.<anonymous> (/libs/cq/gui/components/authoring/editors/clientlibs/core.js:35820:36)
    at Function.each /etc.clientlibs/clientlibs/granite/jquery.js:370:19)
    at jQuery.fn.init.each (/etc.clientlibs/clientlibs/granite/jquery.js:137:17)

Steps to Reproduce

same as "Actual Behavior" above a specific example:

1. go to http://localhost:4502/editor.html/content/we-retail/language-masters/en.html
2. Add an experience fragment component.
3. try to add any other component with the "+" editor action.

Why is this issue occurring:

The issue starts at this line: https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/abefa62b366de3a010471d4022bc9416d79fdfad/content/src/main/content/jcr_root/apps/acs-commons/touchui-widgets/configure-limit-parsys/touchui-limit-parsys.js#L103

Where a call to Granite.author.edit.findEditables() is made which finds all editable on the page. In the process this finds an experience fragment editable then calls ns.util.sanitizeCQHandler on all its configured actions. ns.util.sanitizeCQHandler expects a string but it passed a function causing the error.

It seems like, when the config attribute for the experience fragment editable is parsed, the handler property is mapped to the function instead of keeping it as a string and I have no idea where exactly that mapping is happening.

This would not cause issues typically because when clicking the "+" action on parsys, vanilla AEM does not make a call to Granite.author.edit.findEditables(). Even thought this issue may occur in other parts of the editor, it is amplified by the parsys limiter code.

[ahmed-musallam]

It looks like what is happening is the following:

Assume $cqDom is the cq element dom where the data-config attribute is set.

In adobe's code, a call to $cqDom.data("config") is made, for example:

var config = $cqDom.data("config")

config is now an object that has an actions descendant object. That actions object is sanitized with ns.util.sanitizeCQHandler to basically convert handler strings into functions to be called using eval.

The issue here is that config has been mutated through this process, which means that the jQuery data cache has also been mutated.

So when you make a subsequent call to $cqDom.data("config") you get the mutated config object from jQuery cache and not the parsed object from data-config. Which in turn causes sanitizeCQHandler to fail.

take this simple example:

<div id="data" data-config='{"item1": "val1", "item2":{"child":"value"}}'></div>

var $data = $("#data");
var config = $data.data("config");
console.log(config)
 // line above prints what you'd expect, the object: {"item1": "val1", "item2":{"child":"value"}}
config.item2["hello"] = "hi"
console.log($data.data("config"));
// line above prints the mutated object: {"item1":"val1","item2":{"child":"value","hello":"hi"}}

When adobe fixes this, they need not to mutate the options object, but rather deep clone it and use that object...

[ahmed-musallam]

FYI, I was able to fix this issue with the following code

THIS IS A TEMPORARY FIX! Ie I am not responsible if things break in your environment

Add in a clientlib with categories="[cq.authoring.editor.sites.page]"

(function(ns){
  var originalSanitizeCQHandler = Granite.author.util.sanitizeCQHandler;

  var customSanitizeCQHandler = function (code) {
    // Our added code to fix the issue
    if ($.isFunction(code)) {
      return code;
    } else {
      // cal adobe's original code.
      return originalSanitizeCQHandler(code)
    }
  };

  // override sanitizeCQHandler wth our own impl.
  Granite.author.util.sanitizeCQHandler = function (code) {
    try {
      // try adobe's code first 
      return originalSanitizeCQHandler(code); 
    } catch(error){
       console.warn("Tried to call Granite.author.util.sanitizeCQHandler but failed, using custom fix instead. Please see sourcecode to understand implementation.", error)
       customSanitizeCQHandler(code); // adobe's code failed, use our handler.
    }
  }
})(Granite.author);

[wildone]

@ahmed-musallam thanks for the fix, confirmed works.

[ahmed-musallam]

Closing this issue as I have provided a small fix to mitigate it, but the real fix should come from Adobe in AEM 6.4 SP3 package.

[wildone]

@ahmed-musallam should this fix be added to this package meanwhile?

[ahmed-musallam]

@wildone I added a PR #1546 that slightly fixes the issue, at least you'll be able to add components. As for the JS fix above, that's a temporary fix and it alters the Adobe API. Therefore it should not be added to ACS commons. Like I mentioned above, this issue will be fixed in SP3 package scheduled for release in December.