76966 2012-01-24 17:28:35 -0800 Move opacity clamping into RenderStyle setter. 2012-03-13 20:19:22 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 macpherson macpherson eric macpherson webkit.review.bot oldest_to_newest 541888 0 macpherson 2012-01-24 17:28:35 -0800 Move opacity clamping into RenderStyle setter. 541893 1 123857 macpherson 2012-01-24 17:32:07 -0800 Created attachment 123857 Patch 541913 2 123857 darin 2012-01-24 17:56:12 -0800 Comment on attachment 123857 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. 541940 3 macpherson 2012-01-24 18:18:09 -0800 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. 567430 4 123857 eric 2012-02-28 23:43:03 -0800 Comment on attachment 123857 Patch This removes the error handling.. so now we'll set opacity to 0 on error, no? 567969 5 macpherson 2012-02-29 14:45:31 -0800 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. 578087 6 123857 eric 2012-03-13 18:41:07 -0700 Comment on attachment 123857 Patch LGTM. 578126 7 123857 webkit.review.bot 2012-03-13 20:19:18 -0700 Comment on attachment 123857 Patch Clearing flags on attachment: 123857 Committed r110662: <http://trac.webkit.org/changeset/110662> 578127 8 webkit.review.bot 2012-03-13 20:19:22 -0700 All reviewed patches have been landed. Closing bug. 123857 2012-01-24 17:32:07 -0800 2012-03-13 20:19:18 -0700 Patch bug-76966-20120125123205.patch text/plain 2648 macpherson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNTgzNykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEyLTAxLTI0ICBMdWtlIE1h Y3BoZXJzb24gICA8bWFjcGhlcnNvbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgTW92ZSBvcGFj aXR5IGNsYW1waW5nIGludG8gUmVuZGVyU3R5bGUgc2V0dGVyLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzY5NjYKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDb3ZlcmVkIGJ5IGV4aXN0aW5nIHRlc3RzLgor CisgICAgICAgICogY3NzL0NTU1N0eWxlU2VsZWN0b3IuY3BwOgorICAgICAgICAoV2ViQ29yZTo6 Q1NTU3R5bGVTZWxlY3Rvcjo6YXBwbHlQcm9wZXJ0eSk6CisgICAgICAgICogcmVuZGVyaW5nL3N0 eWxlL1JlbmRlclN0eWxlLmg6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJTdHlsZUJpdGZpZWxk czo6c2V0T3BhY2l0eSk6CisKIDIwMTItMDEtMjQgIEtlbiBCdWNoYW5hbiAgPGtlbnJiQGNocm9t aXVtLm9yZz4KIAogICAgICAgICBDcmFzaCBpbiB1cGRhdGVGaXJzdExldHRlcigpIGZyb20gdW5u ZWNlc3NhcnkgYW5vbnltb3VzIGJsb2NrCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTU3R5 bGVTZWxlY3Rvci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvY3NzL0NTU1N0eWxl U2VsZWN0b3IuY3BwCShyZXZpc2lvbiAxMDU4MTkpCisrKyBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NT U3R5bGVTZWxlY3Rvci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMxNjMsMTEgKzMxNjMsNyBAQCB2 b2lkIENTU1N0eWxlU2VsZWN0b3I6OmFwcGx5UHJvcGVydHkoaW50CiAgICAgICAgIHJldHVybjsK ICAgICB9CiAgICAgY2FzZSBDU1NQcm9wZXJ0eU9wYWNpdHk6Ci0gICAgICAgIEhBTkRMRV9JTkhF UklUX0FORF9JTklUSUFMKG9wYWNpdHksIE9wYWNpdHkpCi0gICAgICAgIGlmICghcHJpbWl0aXZl VmFsdWUgfHwgIXByaW1pdGl2ZVZhbHVlLT5pc051bWJlcigpKQotICAgICAgICAgICAgcmV0dXJu OyAvLyBFcnJvciBjYXNlLgotICAgICAgICAvLyBDbGFtcCBvcGFjaXR5IHRvIHRoZSByYW5nZSAw LTEKLSAgICAgICAgbV9zdHlsZS0+c2V0T3BhY2l0eShjbGFtcFRvPGZsb2F0PihwcmltaXRpdmVW YWx1ZS0+Z2V0RG91YmxlVmFsdWUoKSwgMCwgMSkpOworICAgICAgICBIQU5ETEVfSU5IRVJJVF9B TkRfSU5JVElBTF9BTkRfUFJJTUlUSVZFKG9wYWNpdHksIE9wYWNpdHkpCiAgICAgICAgIHJldHVy bjsKICAgICBjYXNlIENTU1Byb3BlcnR5V2Via2l0Qm94QWxpZ246CiAgICAgICAgIEhBTkRMRV9J TkhFUklUX0FORF9JTklUSUFMX0FORF9QUklNSVRJVkUoYm94QWxpZ24sIEJveEFsaWduKQpJbmRl eDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL3N0eWxlL1JlbmRlclN0eWxlLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL3N0eWxlL1JlbmRlclN0eWxlLmgJKHJldmlz aW9uIDEwNTgxOSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9zdHlsZS9SZW5kZXJTdHls ZS5oCSh3b3JraW5nIGNvcHkpCkBAIC0xMjAyLDcgKzEyMDIsNyBAQCBwdWJsaWM6CiAgICAgdm9p ZCBzZXRUZXh0U3Ryb2tlV2lkdGgoZmxvYXQgdykgeyBTRVRfVkFSKHJhcmVJbmhlcml0ZWREYXRh LCB0ZXh0U3Ryb2tlV2lkdGgsIHcpIH0KICAgICB2b2lkIHNldFRleHRGaWxsQ29sb3IoY29uc3Qg Q29sb3ImIGMpIHsgU0VUX1ZBUihyYXJlSW5oZXJpdGVkRGF0YSwgdGV4dEZpbGxDb2xvciwgYykg fQogICAgIHZvaWQgc2V0Q29sb3JTcGFjZShDb2xvclNwYWNlIHNwYWNlKSB7IFNFVF9WQVIocmFy ZUluaGVyaXRlZERhdGEsIGNvbG9yU3BhY2UsIHNwYWNlKSB9Ci0gICAgdm9pZCBzZXRPcGFjaXR5 KGZsb2F0IGYpIHsgU0VUX1ZBUihyYXJlTm9uSW5oZXJpdGVkRGF0YSwgb3BhY2l0eSwgZik7IH0K KyAgICB2b2lkIHNldE9wYWNpdHkoZmxvYXQgZikgeyBmbG9hdCB2ID0gY2xhbXBUbzxmbG9hdD4o ZiwgMCwgMSk7IFNFVF9WQVIocmFyZU5vbkluaGVyaXRlZERhdGEsIG9wYWNpdHksIHYpOyB9CiAg ICAgdm9pZCBzZXRBcHBlYXJhbmNlKENvbnRyb2xQYXJ0IGEpIHsgU0VUX1ZBUihyYXJlTm9uSW5o ZXJpdGVkRGF0YSwgbV9hcHBlYXJhbmNlLCBhKTsgfQogICAgIC8vIEZvciB2YWxpZCB2YWx1ZXMg b2YgYm94LWFsaWduIHNlZSBodHRwOi8vd3d3LnczLm9yZy9UUi8yMDA5L1dELWNzczMtZmxleGJv eC0yMDA5MDcyMy8jYWxpZ25tZW50CiAgICAgdm9pZCBzZXRCb3hBbGlnbihFQm94QWxpZ25tZW50 IGEpIHsgU0VUX1ZBUihyYXJlTm9uSW5oZXJpdGVkRGF0YS5hY2Nlc3MoKS0+bV9kZXByZWNhdGVk RmxleGlibGVCb3gsIGFsaWduLCBhKTsgfQo=