- Revision
- 176490
- Author
- [email protected]
- 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 <[email protected]>
+
+ [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 <[email protected]>
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 <[email protected]>
+
+ [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 <[email protected]>
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);