Implement Zoom Property in CSSSStyleApplyProperty.
Created attachment 116013 [details] Patch
Could someone please review?
Comment on attachment 116013 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116013&action=review > Source/WebCore/css/CSSStyleApplyProperty.cpp:911 > + } else if (primitiveValue->primitiveType() == CSSPrimitiveValue::CSS_PERCENTAGE) { > + if (float percent = primitiveValue->getFloatValue()) { > + resetEffectiveZoom(selector); > + selector->setZoom(percent / 100.0f); > + } The applyProperty() version of this code would always reset the effective zoom, even if getFloatValue() returns 0. > Source/WebCore/css/CSSStyleApplyProperty.cpp:916 > + } else if (primitiveValue->primitiveType() == CSSPrimitiveValue::CSS_NUMBER) { > + if (float number = primitiveValue->getFloatValue()) { > + resetEffectiveZoom(selector); > + selector->setZoom(number); > + } Same here.
Comment on attachment 116013 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116013&action=review >> Source/WebCore/css/CSSStyleApplyProperty.cpp:911 >> + } > > The applyProperty() version of this code would always reset the effective zoom, even if getFloatValue() returns 0. Hi Andreas, thanks for the review. I understand that's the case in order to exactly match the existing behavior, but is the existing behavior sensible? To my eyes this code treats any zoom of 0 as invalid and does not apply the property on RenderStyle. In that case, do we want zoom: 0; to have a side-effect of resetting the effectiveZoom?
(In reply to comment #4) > (From update of attachment 116013 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116013&action=review > > >> Source/WebCore/css/CSSStyleApplyProperty.cpp:911 > >> + } > > > > The applyProperty() version of this code would always reset the effective zoom, even if getFloatValue() returns 0. > > Hi Andreas, thanks for the review. > I understand that's the case in order to exactly match the existing behavior, but is the existing behavior sensible? > To my eyes this code treats any zoom of 0 as invalid and does not apply the property on RenderStyle. In that case, do we want zoom: 0; to have a side-effect of resetting the effectiveZoom? Is this behavior change detectable by a layout test? If so, we should add one. In general I prefer refactorings to be "stand-alone" and free from behavior changes. A change like this could be pushed as a follow-up patch.
Created attachment 117066 [details] Patch
Comment on attachment 117066 [details] Patch Clearing flags on attachment: 117066 Committed r101499: <http://trac.webkit.org/changeset/101499>
All reviewed patches have been landed. Closing bug.