[bug] Malformed marker payload when marker name is a number

[BrainCrumbz]

Tell us about your environment

• Browser and Browser Version: Chrome 100.0.4896.127 on Windows 11
• After Effects Version: Don't know. Can investigate if needed.

What did you do? Please explain the steps you took before you encountered the problem.

1. Obtain a JSON animation file containing some composition markers, each with a duration, which are named as numbers: 1, 2, 3
2. Integrate lottie-web in vanilla JS page and programmatically load animation into animatedItem. Autoplay is false, loop is false.
3. Upon some user event, programmatically play a specific marker segment through animatedItem.goToAndPlay('1'). At this point, also set loop to true

What did you expect to happen? Requested marker segment should play in loop

What actually happened? Please include as much relevant detail as possible. The whole animation plays in loop.

Please provide a download link to the After Effects file that demonstrates the problem. https://gist.github.com/BrainCrumbz/ce3b2cda9133c3f97d27e79ec2641391#file-marker_20220426_001_numbers_only-json

In case it might help, we'll share here below some details of our investigation.
TA

[BrainCrumbz]

The JSON file definitely has markers, their names are saved as JSON string typed properties, their actual content is a numerical text:

"cm":"1",

When animationItem is loaded, the generated payload property holds the raw number itself:

That seems to be in contrast with the usage of animationItem.markers in getMarkerData function as of today:

if (marker.payload && marker.payload.name.cm === markerName) {

In order to recognize marker name, it will check for a structured payload, not a number right in the payload. In a previous test, also launching animatedItem.goToAndPlay(1), passing a raw number, had no effect.

Just for completeness, we then tried with a different JSON animation file, which has markers named both with a generic text or with a number:

https://gist.github.com/BrainCrumbz/ce3b2cda9133c3f97d27e79ec2641391#file-marker_20220426_001_names_numbers-json

"cm":"exit",
...
"cm":"4",

When animationItem is loaded, the generated payload property still holds the raw number itself for the number case, but correctly holds a structured object for the generic text case:

When launching animatedItem.goToAndPlay('exit'), only the marker segment is correctly played (in loop as requested).

[BrainCrumbz]

Finally, all that seems to boil down to markerParser function:

        try {
          markerData.payload = JSON.parse(_markers[i].cm);
        } catch (_) {
          try {
            markerData.payload = parsePayloadLines(_markers[i].cm);
          } catch (__) {
            markerData.payload = {
              name: _markers[i]
            };
          }
        }

where:

• JSON.parse('abc') and parsePayloadLines('abc') throw, given that's wrong JSON syntax
• while JSON.parse('1') does not throw, givent that's correct JSON syntax and confuses the marker parsing logic.

A similar problem should happend when one names a marker as 'true' or 'false', as that will again be correct JSON syntax

[BrainCrumbz]

Would you consider a PR addressing this issue? E.g. with an approach like the following:

let tempPayload;

try {
  tempPayload = JSON.parse(_markers[i].cm);
} catch (_) {
  try {
    tempPayload = parsePayloadLines(_markers[i].cm);
  } catch (__) {
    tempPayload = {
      name: _markers[i]
    };
  }
}

// ...here there would be a comment explaining why...
if (typeof tempPayload !== 'object') {
  tempPayload = {
    name: _markers[i].cm
  };
}

markerData.payload = tempPayload;

[BrainCrumbz]

Hi, sorry, could you please rephrase? Cannot understand the previous comment. Thanks

[bodymovin]

Hey thanks for the detailed info. I've been looking into it and this indeed fixes markers configuration. I think that it can also be solved by checking isNaN(_markers[i].cm)
If it is false, set the payload manually.
But the goToAndPlay and goToAndStop methods need an update as well since they are assuming that anything that can be cast as a Number is a frame and not a marker.
I can work on fixing it, but let me know if you want to address both issues together.

[BrainCrumbz]

What about when one names a marker as true or false ?

We understand this may seems like a bail out, but if there are more changes involved, how would we ensure we're not breaking anything?
That kinda worries also because there are 3 supported ways to set a marker name (and description): not sure what option could be broken by those changes.

[bodymovin]

I don't think supporting markers as booleans is needed. Regarding breaking things, I don't know what are users expecting if they're naming their markers as numbers, but that's not working already, so doing this fix should at most improve the behavior.

[BrainCrumbz]

What marker name values do you think should be used as test cases? Will try to list some here:

"cm":"123",

"cm":"abc",

"cm":"abc\r\n",

"cm":"name: abc\r\ndescription: def",

"cm":"name: 123\r\ndescription: def",

"cm":"{\r\n  \"name\":\"abc\",\r\n  \"description\": \"def\"\r\n}",

"cm":"{\r\n  \"name\":\"123\",\r\n  \"description\": \"def\"\r\n}",

Are there more?