1057900 - test_value_cloning.html and test_value_computation.html fail with CSS filters enabled

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Max Vujovic]

If you run:
./mach mochitest-plain --setpref=layout.css.filters.enabled=true

You get the following failures:

248009 WARNING The following tests failed:
248010 INFO TEST-UNEXPECTED-ERROR | /tests/layout/style/test/test_value_cloning.html | Assertion count 12 is greater than expected range 0-0 assertions.
TEST-INFO expected OK
248011 INFO TEST-UNEXPECTED-ERROR | /tests/layout/style/test/test_value_computation.html | Assertion count 13 is greater than expected range 9-9 assertions.
TEST-INFO expected OK
248012 INFO SUITE-END | took 1110s

I'll be looking into this. I don't remember seeing any mochitest failures when CSS filter parsing was landed some months ago, so this might be interaction with new code- we'll see.

Comment 1

[Max Vujovic]

The assertions are happening on cases like "blur(calc(10%))". We're parsing calc values for CSS filter lengths, but we're not resolving them yet. I'll work on that.

Comment 2

[Max Vujovic]

(In reply to Max Vujovic from comment #1)
> The assertions are happening on cases like "blur(calc(10%))".

I mean "blur(calc(1px))"- percentage values are correctly rejected at parse time, since blur should only take a <length>.

Comment 3

[Max Vujovic]

Attachment: Patch

r? dbaron for nsRuleNode changes.
r? mstange for rendering reftests.

Before this patch, a calc() argument in CSS blur() would be stored and then not handled in nsCSSFilterInstance.cpp. This resulted in an "unexpected unit" assertion in nsCSSFilterInstance::SetAttributesForBlur, which expected only eStyleUnit_Coord (not eStyleUnit_Calc). Now, calc() is resolved to a coord in nsRuleNode, using the same flags as the blur radius for box/drop shadows [1].

I added two rendering tests, one for a postive calc value and one for a negative calc value because I thought it'd be nice to know that the rendering was correct in addition to the parsing tests.

[1]: http://dxr.mozilla.org/mozilla-central/source/layout/style/nsRuleNode.cpp?from=nsRuleNode.cpp#4060

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8478680 [details] [diff] [review]
Patch

r=dbaron on nsRuleNode changes

Comment 5

[Max Vujovic]

Comment on attachment 8478680 [details] [diff] [review]
Patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/efd173e4cc23
https://tbpl.mozilla.org/?tree=Try&rev=4489b8bb1beb

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/efd173e4cc23