Amphtml: amp-gwd-animation: repeated gotoAndPlay does not work

[sklobovskaya]

The compiler appears to be stripping out code which forces layout reflow between removing and adding CSS classes, which is breaking repeated "go to label" animation controls.

Source:
The function at https://github.com/ampproject/amphtml/blob/master/extensions/amp-gwd-animation/0.1/amp-gwd-animation-impl.js#L417

Compiled (previously):

a.classList.remove("gwd-pause-animation");var c=a.getAttribute("data-gwd-label-animation");
c&&(a.classList.remove(c),a.removeAttribute("data-gwd-label-animation"));
c==b&&(a.offsetWidth=a.offsetWidth);
a.classList.add(b);a.setAttribute("data-gwd-label-animation",b)

Compiled (now):

a.classList.remove("gwd-pause-animation");var c=a.getAttribute("data-gwd-label-animation");
c&&(a.classList.remove(c),a.removeAttribute("data-gwd-label-animation"));
a.classList.add(b);a.setAttribute("data-gwd-label-animation",b)

c==b&&(a.offsetWidth=a.offsetWidth); is no longer present in the compiled output. This corresponds to lines 432-434 in the source.

This can be reproduced in an AMP document using amp-gwd-animation by creating an animated element that has a label at 0s and a gotoAndPlay event marker at the end of the animation which goes to the label (creating an infinite loop between the label and the event marker).

[sklobovskaya]

@dvoytenko Hi Dima, do you know if this is a recent change? Is there some updated way to annotate that line of code to prevent it from being stripped out, or do we need to resort to a different approach?

[alanorozco]

@erwinmombay Any ideas on why it might be stripping the code and how to mitigate it?
@sklobovskaya A snippet comparing the compiled output vs the input would be useful :)

[sklobovskaya]

Updated description with the input and output.

[alanorozco]

@rsimha I realized this might be caused by upgrading Closure?

[rsimha]

@alanorozco, could it be that the code needs to get annotated differently so it doesn't get stripped out?

/cc @erwinmombay

[alanorozco]

Based on this it looks like there's no annotation to accomplish what @rsimha is mentioning.

Closure thinks there aren't any side effects since the value is not assigned anywhere. So the solution I can come up with is tricking Closure to think we're exporting the value read.

function reflow(element) {
  const value = element./*OK*/offsetWidth;
  // exporting global to trick Closure into thinking this function has side
  // effects.
  const globalRef = '__AMP_TEMP';
  self[globalRef] = value;
  delete self[globalRef];
  return value;
}

This outputs the following:

a.classList.remove("gwd-pause-animation");
var c = a.getAttribute("data-gwd-label-animation");
c && (a.classList.remove(c), a.removeAttribute("data-gwd-label-animation"));
c == b && (self.__AMP_TEMP = a.offsetWidth, delete self.__AMP_TEMP);
a.classList.add(b);
a.setAttribute("data-gwd-label-animation", b)

We can see that the reflow remains in a.offsetWidth.

I filed #19367 in case this seems like a reasonable solution to you @sklobovskaya :)

[sklobovskaya]

Yeah, creating some fake side effects seems fine if there's no appropriate annotation. (LGTM on your PR) Thanks for the quick fix!