Skip to content

Conversation

@bramus
Copy link
Collaborator

@bramus bramus commented Dec 23, 2023

In #178 parseAnimationRange was changed to use string values of "normal" instead of objects representing the normal range. This essentially broke the parsing of animation-range, as later on that function tries to update values in that (now inexistent) object.

On the console, these errors could be seen, as reported in #183:

Can’t assign to property "rangeName": not an object

This PR fixes this: return the default "normal" when no value is given, but continue with the objects when there is a value to parse.

Note that this is only for parsing ranges of ViewTimelime instances. For ScrollTimeline, more work needs to be done in #153.

PS: I didn’t get to updating the test results as I’ve got issues launching them with firefox for some reason. Safari works fine, but gives totally different base results.

Fixes the error “Can’t assign to property "rangeName": not an object”
@bramus bramus mentioned this pull request Dec 23, 2023
@johannesodland
Copy link
Contributor

I'm so sorry.

I think it might be better to just roll back #178.

I was too optimistic when rebasing on top of the recent changes, and leaned too heavily on the WPT test result being ok.

If not rolled back, #187 should probably be merged as well, as there is an issue matching the range names entry-crossing and exit-crossing.

@bramus
Copy link
Collaborator Author

bramus commented Dec 24, 2023

I’ll get this one and #178 in, as #178 contained some other good changes.

In #153 the range parsing will undergo a rewrite, so I think the current code is fine – it will get refactored anyways.

@bramus bramus merged commit 945a8ac into flackr:master Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants