[Webkit-unassigned] [Bug 98950] Bad copy constructor in StyleRareNonInheritedData

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=98950


Chiculita Alexandru <achicu at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |achicu at adobe.com




--- Comment #12 from Chiculita Alexandru <achicu at adobe.com>  2012-10-11 15:03:39 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=168282&action=review

Looks good! I think the test could be simplified.

> LayoutTests/platform/chromium/TestExpectations:110
> +# Fails because chromium doesn't have blendmode

I think it is better to just say that Blend Mode is not enabled on Chromium yet :)

> LayoutTests/transitions/blendmode-transitions.html:17
> +      -webkit-transition-duration: 1s;
> +      -webkit-transition-timing-function: linear;
> +      -webkit-transition-property: -webkit-box-shadow, text-shadow;
> +	  -webkit-blend-mode: difference;

Why not just have a simple test to check the computed style before and after the change? This looks like it could be a simple JS test instead. Anyway, using a transition seems a little too much :)

> LayoutTests/transitions/blendmode-transitions.html:33
> +      -webkit-box-shadow: 0 -20px 10px red, 0 20px 10px blue;
> +      text-shadow: 0 -20px 10px red, 0 20px 10px blue;

Do you really need the shadows?

> LayoutTests/transitions/resources/transition-test-helpers.js:166
> +                case CSSPrimitiveValue.CSS_IDENT:

I think you are still not testing this one. You should pass in an arbitrary string and see what happens and have it as a bad parsing test (also a JS only test could handle this).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.