1366197 - interpolation bug in caret-color

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Florian Rivoal]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170504105526

Steps to reproduce:

Open this test of the CSSWG test suite for the CSS caret-color property

https://test.csswg.org/suites/css-ui-3_dev/nightly-unstable/html/caret-color-020.htm


Actual results:

The test fails.

However, while the test is trying to confirm that color interpolation during animation happens, the failure is not due to no interpolation at all, but rather to the interpolation resulting in an unexpected number.

This problem does not reproduce in the another similar test:
https://test.csswg.org/suites/css-ui-3_dev/nightly-unstable/html/caret-color-018.htm



Expected results:

The test should pass.

Discussing this over email with Xidorn Quan, he said:

> That inconsistency could be a bug of Gecko... I can understand how this happens: for interpolation between numeric color and currentcolor, we use an 8-bit unsigned integer to store the ratio, which means that the ratio is already rounded before combining the two values, which means in any moment, the decreasing value and the increasing value would not be equal. The value could become rgb(128, 127, 128) or rgb(127, 128, 127) but never rgb(128, 128, 128)...

> To get rgb(128, 128, 128), the ratio between numeric color and currentcolor needs to be stored in a higher precision, which... we probably can do. Please file a bug to Gecko and cc me there. It should probably be in Core :: CSS Parsing and Computation component, and it should probably block bug 1299741 rather than caret-color.

Comment 1

[Florian Rivoal]

This is currently the last thing in the CSS-UI test suite not to have 2 implementations (except things marked "at-risk" and things with "should" rather than "must" requirements). It would be lovely if you could fix this.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

It's worth checking whether it has already been fixed in Stylo, our new style system. I suspect it is, because IIRC we only do float interpolation for color. In that case, I'm not keen to fix it in the old style system, which is expected to sunset in 57.

But in that case, this may be a behavior change we want to track for stylo.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

ni? myself for checking that when I have a stylo build.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Thought about this on flight, and found I was wrong. This is because of the precision of foreground ratio. There is nothing about how interpolation is implemented. So it would be some part shared between the new and old style system which wouldn't be fixed magically...

Comment 5

[Emilio Cobos Álvarez [:emilio]]

This test is working now, should we close this? We interpolate the foreground ratio as a float now.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Attachment: caret-color-020.html

The test has definitely been changed since then, and it no longer checks the exact value during interpolation. This attachment is the old version of this test when the bug was initially filed.

I've checked that Stylo still fails on this test.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Attachment: testcase

This is a simplified testcase which can run without wpt harness. It should show "rgb(128, 128, 128)" but currently show "rgb(128, 127, 128)".

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

As I've mentioned in comment 4, this is because of the precision of mForegroundRatio, not something related to interpolation, so it's not going to fixed automatically by the new style system.