Title: [176490] trunk
Revision
176490
Author
cdu...@apple.com
Date
2014-11-21 17:22:41 -0800 (Fri, 21 Nov 2014)

Log Message

[iOS] Regression(r176202): line-height is wrong on marco.org
https://bugs.webkit.org/show_bug.cgi?id=138970

Reviewed by Simon Fraser.

Source/WebCore:

After r176202, on iOS with IOS_TEXT_AUTOSIZING enabled, we would
multiply the lineHeight by RenderStyle::textSizeAdjust()::multiplier()
unconditionally. However, we're only supposed to do so if
RenderStyle::textSizeAdjust()::isPercentage() returns true. This
patch reintroduces the textSizeAdjust().isPercentage() check that was
inadvertently dropped when refactoring the code to be shared between
iOS and OS X.

Additionally, the multiplier is only supposed the be applied if the
input CSSPrimitiveValue is a Length or a Number. However, after
r176202, we would apply the multiplier if the CSSPrimitiveValue is
a Percentage or a Number. This patch updates the code to match the
behavior prior to r176202.

Test: fast/css/line-height-text-autosizing.html

* css/StyleBuilderCustom.h:
(WebCore::StyleBuilderFunctions::convertLineHeight):
(WebCore::StyleBuilderFunctions::applyValueLineHeight):

LayoutTests:

Add layout test to cover line-height CSS property and its interaction
with -webkit-text-size-adjust.

* fast/css/line-height-text-autosizing-expected.txt: Added.
* fast/css/line-height-text-autosizing.html: Added.
* platform/ios-simulator/fast/css/line-height-text-autosizing-expected.txt: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (176489 => 176490)


--- trunk/LayoutTests/ChangeLog	2014-11-22 01:09:21 UTC (rev 176489)
+++ trunk/LayoutTests/ChangeLog	2014-11-22 01:22:41 UTC (rev 176490)
@@ -1,3 +1,17 @@
+2014-11-21  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] Regression(r176202): line-height is wrong on marco.org
+        https://bugs.webkit.org/show_bug.cgi?id=138970
+
+        Reviewed by Simon Fraser.
+
+        Add layout test to cover line-height CSS property and its interaction
+        with -webkit-text-size-adjust.
+
+        * fast/css/line-height-text-autosizing-expected.txt: Added.
+        * fast/css/line-height-text-autosizing.html: Added.
+        * platform/ios-simulator/fast/css/line-height-text-autosizing-expected.txt: Added.
+
 2014-11-21  Chris Fleizach  <cfleiz...@apple.com>
 
         AX: Unclear that user and password are autofilled, no VoiceOver version of the yellow outline.

Added: trunk/LayoutTests/fast/css/line-height-text-autosizing-expected.txt (0 => 176490)


--- trunk/LayoutTests/fast/css/line-height-text-autosizing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/line-height-text-autosizing-expected.txt	2014-11-22 01:22:41 UTC (rev 176490)
@@ -0,0 +1,21 @@
+Test the 'line-height' property interaction with '-webkit-text-size-adjust'.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Case without text size adjust.
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "normal"
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('font-size') is "20px"
+testDivNoAdjust.style['line-height'] = '1.6'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "32px"
+testDivNoAdjust.style['line-height'] = '80%'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "16px"
+testDivNoAdjust.style['line-height'] = '12px'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "12px"
+Case with text size adjust.
+Platform does not support  -webkit-text-size-adjust
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Test
+Test

Added: trunk/LayoutTests/fast/css/line-height-text-autosizing.html (0 => 176490)


--- trunk/LayoutTests/fast/css/line-height-text-autosizing.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/line-height-text-autosizing.html	2014-11-22 01:22:41 UTC (rev 176490)
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+#testDivAdjust {
+  -webkit-text-size-adjust: 150%;
+  font-size: 20px;
+}
+#testDivNoAdjust {
+  font-size: 20px;
+}
+</style>
+</head>
+<boby>
+<div id="testDivNoAdjust">Test</div>
+<div id="testDivAdjust">Test</div>
+<script>
+description("Test the 'line-height' property interaction with '-webkit-text-size-adjust'.");
+
+debug("Case without text size adjust.");
+var testDivNoAdjust = document.getElementById("testDivNoAdjust");
+shouldBeEqualToString("window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height')", "normal");
+shouldBeEqualToString("window.getComputedStyle(testDivNoAdjust).getPropertyValue('font-size')", "20px");
+evalAndLog("testDivNoAdjust.style['line-height'] = '1.6'");
+// font-size * line-height == 20px * 1.6 == 32px
+shouldBeEqualToString("window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height')", "32px");
+evalAndLog("testDivNoAdjust.style['line-height'] = '80%'");
+// font-size * line-height == 20px * 80% == 16px
+shouldBeEqualToString("window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height')", "16px");
+evalAndLog("testDivNoAdjust.style['line-height'] = '12px'");
+shouldBeEqualToString("window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height')", "12px");
+
+debug("Case with text size adjust.");
+var testDivAdjust = document.getElementById("testDivAdjust");
+if (window.getComputedStyle(testDivAdjust).getPropertyValue('-webkit-text-size-adjust') == null) {
+  debug("Platform does not support  -webkit-text-size-adjust");
+} else {
+  debug("Platform supports -webkit-text-size-adjust");
+  shouldBeEqualToString("window.getComputedStyle(testDivAdjust).getPropertyValue('line-height')", "normal");
+  // font-size * -webkit-text-size-adjust == 20px * 150% == 30px
+  shouldBeEqualToString("window.getComputedStyle(testDivAdjust).getPropertyValue('font-size')", "30px");
+  evalAndLog("testDivAdjust.style['line-height'] = '1.6'");
+  // font-size * line-height == 30px * 1.6 == 48px
+  shouldBeEqualToString("window.getComputedStyle(testDivAdjust).getPropertyValue('line-height')", "48px");
+
+  evalAndLog("testDivAdjust.style['line-height'] = '80%'");
+  // font-size * line-height == 30px * 80% == 24px
+  shouldBeEqualToString("window.getComputedStyle(testDivAdjust).getPropertyValue('line-height')", "24px");
+
+  evalAndLog("testDivAdjust.style['line-height'] = '12px'");
+  // line-height * -webkit-text-size-adjust == 12px * 150% == 18px
+  shouldBeEqualToString("window.getComputedStyle(testDivAdjust).getPropertyValue('line-height')", "18px");
+}
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (176489 => 176490)


--- trunk/LayoutTests/platform/ios-simulator/TestExpectations	2014-11-22 01:09:21 UTC (rev 176489)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations	2014-11-22 01:22:41 UTC (rev 176490)
@@ -151,3 +151,8 @@
 traversal
 userscripts
 webarchive
+
+###
+# Mark as passing specific tests in folders that were skipped temporarily above.
+##
+webkit.org/b/138970 fast/css/line-height-text-autosizing.html [ Pass ]

Added: trunk/LayoutTests/platform/ios-simulator/fast/css/line-height-text-autosizing-expected.txt (0 => 176490)


--- trunk/LayoutTests/platform/ios-simulator/fast/css/line-height-text-autosizing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/ios-simulator/fast/css/line-height-text-autosizing-expected.txt	2014-11-22 01:22:41 UTC (rev 176490)
@@ -0,0 +1,29 @@
+Test the 'line-height' property interaction with '-webkit-text-size-adjust'.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Case without text size adjust.
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "normal"
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('font-size') is "20px"
+testDivNoAdjust.style['line-height'] = '1.6'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "32px"
+testDivNoAdjust.style['line-height'] = '80%'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "16px"
+testDivNoAdjust.style['line-height'] = '12px'
+PASS window.getComputedStyle(testDivNoAdjust).getPropertyValue('line-height') is "12px"
+Case with text size adjust.
+Platform supports -webkit-text-size-adjust
+PASS window.getComputedStyle(testDivAdjust).getPropertyValue('line-height') is "normal"
+PASS window.getComputedStyle(testDivAdjust).getPropertyValue('font-size') is "30px"
+testDivAdjust.style['line-height'] = '1.6'
+PASS window.getComputedStyle(testDivAdjust).getPropertyValue('line-height') is "48px"
+testDivAdjust.style['line-height'] = '80%'
+PASS window.getComputedStyle(testDivAdjust).getPropertyValue('line-height') is "24px"
+testDivAdjust.style['line-height'] = '12px'
+PASS window.getComputedStyle(testDivAdjust).getPropertyValue('line-height') is "18px"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Test
+Test

Modified: trunk/Source/WebCore/ChangeLog (176489 => 176490)


--- trunk/Source/WebCore/ChangeLog	2014-11-22 01:09:21 UTC (rev 176489)
+++ trunk/Source/WebCore/ChangeLog	2014-11-22 01:22:41 UTC (rev 176490)
@@ -1,3 +1,30 @@
+2014-11-21  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] Regression(r176202): line-height is wrong on marco.org
+        https://bugs.webkit.org/show_bug.cgi?id=138970
+
+        Reviewed by Simon Fraser.
+
+        After r176202, on iOS with IOS_TEXT_AUTOSIZING enabled, we would
+        multiply the lineHeight by RenderStyle::textSizeAdjust()::multiplier()
+        unconditionally. However, we're only supposed to do so if
+        RenderStyle::textSizeAdjust()::isPercentage() returns true. This
+        patch reintroduces the textSizeAdjust().isPercentage() check that was
+        inadvertently dropped when refactoring the code to be shared between
+        iOS and OS X.
+
+        Additionally, the multiplier is only supposed the be applied if the
+        input CSSPrimitiveValue is a Length or a Number. However, after
+        r176202, we would apply the multiplier if the CSSPrimitiveValue is
+        a Percentage or a Number. This patch updates the code to match the
+        behavior prior to r176202.
+
+        Test: fast/css/line-height-text-autosizing.html
+
+        * css/StyleBuilderCustom.h:
+        (WebCore::StyleBuilderFunctions::convertLineHeight):
+        (WebCore::StyleBuilderFunctions::applyValueLineHeight):
+
 2014-11-21  Chris Fleizach  <cfleiz...@apple.com>
 
         AX: com.apple.WebKit.WebContent crashed at WebCore: WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const

Modified: trunk/Source/WebCore/css/StyleBuilderCustom.h (176489 => 176490)


--- trunk/Source/WebCore/css/StyleBuilderCustom.h	2014-11-22 01:09:21 UTC (rev 176489)
+++ trunk/Source/WebCore/css/StyleBuilderCustom.h	2014-11-22 01:22:41 UTC (rev 176490)
@@ -521,11 +521,13 @@
     }
     if (primitiveValue.isLength()) {
         length = primitiveValue.computeLength<Length>(csstoLengthConversionDataWithTextZoomFactor(styleResolver));
+        if (multiplier != 1.f)
+            length = Length(length.value() * multiplier, Fixed);
         return true;
     }
     if (primitiveValue.isPercentage()) {
         // FIXME: percentage should not be restricted to an integer here.
-        length = Length((styleResolver.style()->computedFontSize() * primitiveValue.getIntValue()) * multiplier / 100, Fixed);
+        length = Length((styleResolver.style()->computedFontSize() * primitiveValue.getIntValue()) / 100, Fixed);
         return true;
     }
     if (primitiveValue.isNumber()) {
@@ -571,7 +573,8 @@
 inline void applyValueLineHeight(StyleResolver& styleResolver, CSSValue& value)
 {
     Length lineHeight;
-    if (!convertLineHeight(styleResolver, value, lineHeight, styleResolver.style()->textSizeAdjust().multiplier()))
+    float multiplier = styleResolver.style()->textSizeAdjust().isPercentage() ? styleResolver.style()->textSizeAdjust().multiplier() : 1.f;
+    if (!convertLineHeight(styleResolver, value, lineHeight, multiplier))
         return;
 
     styleResolver.style()->setLineHeight(lineHeight);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to