Move opacity clamping into RenderStyle setter.

[Luke Macpherson]

Move opacity clamping into RenderStyle setter.

[Luke Macpherson]

Created attachment 123857 [details]
Patch

[Darin Adler]

Comment on attachment 123857 [details]
Patch

Is this really the design we want going forward? RenderStyle was a sort of data holder without behavior. This goes in a different direction.

[Luke Macpherson]

This is a convenient place to enforce the invariant. I guess an alternate approach would be to introduce an Opacity type and have that enforce the value between 0 and 1, but that requires significantly more code.

[Eric Seidel (no email)]

Comment on attachment 123857 [details]
Patch

This removes the error handling.. so now we'll set opacity to 0 on error, no?

[Luke Macpherson]

That "error case" is unreachable.
CSSParser.cpp:1653: case CSSPropertyOpacity:
CSSParser.cpp:1654: validPrimitive = validUnit(value, FNumber, m_strict);
So the parser can only generate numbers for this property.

If it existed at all, it should have been an assertion, but most of the code in ::applyProperty() assumes that the parser has only passed in valid types at this point.

[Eric Seidel (no email)]

Comment on attachment 123857 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 123857 [details]
Patch

Clearing flags on attachment: 123857

Committed r110662: <http://trac.webkit.org/changeset/110662>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.