New issue
Advanced search Search tips

Issue 913458 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 873101



Sign in to add a comment

Result of animating css-logical properties is not deterministic

Project Member Reported by andruud@chromium.org, Dec 10

Issue description

StyleResolver::ApplyAnimatedStandardProperties applies active interpolations by iterating the contents of a HashMap. This means that the application order of interpolations isn't defined, and will vary according to the return value of PropertyHandle::GetHash.

This is a problem for tests like wpt/css/css-logical/animation-001.html, which expects animations like the following to have a stable result:

  element.animate({
    marginLeft: '200px',
    marginInline: '100px'
  },
  { duration: 1000, easing: 'step-start' });

Currently in Blink, the computed value for 'margin-left' will either be "100px", or "200px", depending on how entries appear in the HashMap.

I'm not sure if this is a spec issue for css-logical, or if we should change the application of animated values to be deterministic (or both).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4a7f9c6b6843e98a3e7fe53a66c398e618bd46c

commit d4a7f9c6b6843e98a3e7fe53a66c398e618bd46c
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Mon Dec 10 17:15:46 2018

Ignore non-deterministic test animation-001.html.

StyleResolver::ApplyAnimatedStandardProperties does not guarantee that
active interpolations are applied in any specific order. This means that
animations involving both flow-relative properties and equivalent
physical properties produce non-deterministic computed values.

This was discovered when adding a new (entirely unrelated) CSS property,
which generated a new entry in the CSSPropertyID enum, which ultimately
resulted in a different value for PropertyHandle::GetHash for subsequent
properties (enum-wise) properties, and a different iteration order for the
map of active interpolations.

Bug: 913458
Change-Id: I022e106664e2bcadf65a1d846ded477e0df54a85
Reviewed-on: https://chromium-review.googlesource.com/c/1370168
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615151}
[modify] https://crrev.com/d4a7f9c6b6843e98a3e7fe53a66c398e618bd46c/third_party/blink/web_tests/TestExpectations
[delete] https://crrev.com/f4a3f14769d5e7e338d86116659cc8e440663671/third_party/blink/web_tests/external/wpt/css/css-logical/animation-001-expected.txt

Labels: Hotlist-Polish
The test claims that "Physical longhands win over logical shorthands" but is this true? css-logical has a passage that the overriding behavior is not determined based on whether the propery is physical or logical which seems to contradict the test [1] 

In either case, I don't think the indeterministic behavior is correct. I am  marking this Hotlist-polish since it seems like a bug in Blink regardless of whether the test is correct or not.




[1] See https://drafts.csswg.org/css-logical/#box

"A computed value that has flow-relative and physical properties as input is determined by applying the CSS cascade to declarations of both. Overriding is not determined by whether a declaration is flow-relative or physical, but only by the rules of the CSS cascade"
Hey, test reviewer here :)

That test is based on:

  https://drafts.csswg.org/web-animations/#calculating-computed-keyframes

In particular:

> If conflicts arise when expanding shorthand properties or replacing logical properties with physical propertiess, apply the following rules in order until the conflict is resolved:
>
> [...]
>
> Physical properties override logical properties.

Thanks majidvp, emilio. Then this definitely looks like a bug in Blink ...
Blocking: 873101
Yes, logical properties are not interpolated properly in CSS Animations nor Web Animations. I already filed issue 873101, but I only fixed the CSS Transitions case, the other ones are trickier.

Sign in to add a comment