RESOLVED FIXED 76966
Move opacity clamping into RenderStyle setter.
https://bugs.webkit.org/show_bug.cgi?id=76966
Summary Move opacity clamping into RenderStyle setter.
Luke Macpherson
Reported 2012-01-24 17:28:35 PST
Move opacity clamping into RenderStyle setter.
Attachments
Patch (2.59 KB, patch)
2012-01-24 17:32 PST, Luke Macpherson
no flags
Luke Macpherson
Comment 1 2012-01-24 17:32:07 PST
Darin Adler
Comment 2 2012-01-24 17:56:12 PST
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
Comment 3 2012-01-24 18:18:09 PST
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 4 2012-02-28 23:43:03 PST
Comment on attachment 123857 [details] Patch This removes the error handling.. so now we'll set opacity to 0 on error, no?
Luke Macpherson
Comment 5 2012-02-29 14:45:31 PST
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 6 2012-03-13 18:41:07 PDT
Comment on attachment 123857 [details] Patch LGTM.
WebKit Review Bot
Comment 7 2012-03-13 20:19:18 PDT
Comment on attachment 123857 [details] Patch Clearing flags on attachment: 123857 Committed r110662: <http://trac.webkit.org/changeset/110662>
WebKit Review Bot
Comment 8 2012-03-13 20:19:22 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.