Handle exceptions when auto-aligning start time

[johannesodland]

It is currently possible to set values for the attachment range that are valid length-percentage values, but that are not supported in the polyfill yet.

This will cause en error to be thrown when we try to auto-align the start time, preventing tickAnimation from completing as expected.

We should probably add a stricter validation when setting the range, but for now I think we should catch the exception and fall back to using the full animation range.

Merging this PR should prevent the timeouts that occurred in the WPT subtests in #153

[bramus]

It is currently possible to set values for the attachment range that are valid length-percentage values, but that are not supported in the polyfill yet.

What are some examples of these values?

Merging this PR should prevent the timeouts that occurred in the WPT subtests in #153

Perfect!

[johannesodland]

What are some examples of these values?

Currently we don’t support relative units such as em or vh as we can not resolve them. They will parse fine as length-percentage values, but will not resolve to px values, yet.

Edit: I realized I didn't add any examples, so I'm adding one now.

element.animate( { width: ['0px', '200px' ] }, {
      timeline: viewTimeline,
      rangeStart: '5em',
      rangeEnd: 'calc(1px + 10vh),
      fill: 'both'
    });

Both rangeStart and rangeEnd are <length-percentage> values, but atm they are not supported. We don't validate the values upon setting/parsing them, but trying to resolve the values when we need to use them will throw an exception.

[bramus]

Currently we don’t support relative units such as em or vh as we can not resolve them. They will parse fine as length-percentage values, but will not resolve to px values, yet.

Maybe we should add a console.warn() to notify the developer about this. This would be helpful to help them debug, thereby reducing frustration along the way.

[johannesodland]

Maybe we should add a console.warn() to notify the developer about this. This would be helpful to help them debug, thereby reducing frustration along the way.

Maybe 🤔

I considered this, but this procedure can be run quite often and it would spam the logs. Maybe if we add a flag per timeline and only issue the warning once? 🤔

[bramus]

I considered this, but this procedure can be run quite often and it would spam the logs.

Hmm, good call. Spamming the console with message is infeasible indeed.

Maybe if we add a flag per timeline and only issue the warning once?

Another way to bypass this could be to error out entirely for that specific Scroll-Timeline: instead of adding an animation with the default (wrong) timeline offset, remove the scroll-animation altogether.

[johannesodland]

Another way to bypass this could be to error out entirely for that specific Scroll-Timeline: instead of adding an animation with the default (wrong) timeline offset, remove the scroll-animation altogether.

Hmmm.. I think we can leave the timeline active, as the range is set and calculated for the animation and not the timeline. The timeline might be used for multiple animations, and we should leave the others playing.

I think we can either

1. cancel the current animation after a console.warn()
2. reset the animation range start/end to the default "normal" after a console.warn(), leaving the animation playing.

I'm not sure which would be the best, but I'm slightly leaning towards the latter.
It could look something like this:

  try {
    startOffset = CSS.percent(fractionalStartDelay(details) * 100);
  } catch (e) {
    // TODO: Validate supported values for range start, to avoid exceptions when resolving the values.

    // Range start is invalid, falling back to default value
    startOffset = CSS.percent(0);
    details.animationRange.start = 'normal';
    console.warn("Exception when calculating start offset", e);
  }

It would be better to validate the range when it's set. It might be hard to detect whether a CSSNumericValue contains unsupported units, but we might be able to leverage CSSNumericValue toSum() to check which units are used. This is something we should solve in a separate PR though.

[johannesodland]

@bramus I implemented the change I suggested in the comment above.