Text flow broken in elements with vertical align top/bottom and inline elements talle...
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 May 2013 12:08:57 +0000 (12:08 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 May 2013 12:08:57 +0000 (12:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=111974

Source/WebCore:

Reviewed by Ryosuke Niwa.

Per http://www.w3.org/TR/CSS2/visudet.html#propdef-vertical-align 'vertical-align' only applies to inline and table-cell
elements.

Test: fast/css/vertical-align-block-elements.html

* rendering/InlineFlowBox.cpp:
(WebCore::isTextInBlockElement):
(WebCore):
(WebCore::InlineFlowBox::adjustMaxAscentAndDescent):
(WebCore::InlineFlowBox::computeLogicalBoxHeights):
(WebCore::InlineFlowBox::placeBoxesInBlockDirection):

LayoutTests:

Reviewed by Ryosuke Niwa.

* editing/execCommand/query-command-state-expected.txt:
* editing/execCommand/script-tests/query-command-state.js: Remove invalid tests, vertical-align does not apply to
  div elements.
(runTests):
* fast/css/vertical-align-block-elements-expected.html: Added.
* fast/css/vertical-align-block-elements.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@149929 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/query-command-state-expected.txt
LayoutTests/editing/execCommand/script-tests/query-command-state.js
LayoutTests/fast/css/vertical-align-block-elements-expected.html [new file with mode: 0644]
LayoutTests/fast/css/vertical-align-block-elements.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineFlowBox.cpp

index 01f485d..6dab978 100644 (file)
@@ -1,3 +1,17 @@
+2013-03-17  Robert Hogan  <robert@webkit.org>
+
+        Text flow broken in elements with vertical align top/bottom and inline elements taller than line-height
+        https://bugs.webkit.org/show_bug.cgi?id=111974
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/execCommand/query-command-state-expected.txt:
+        * editing/execCommand/script-tests/query-command-state.js: Remove invalid tests, vertical-align does not apply to 
+          div elements.
+        (runTests):
+        * fast/css/vertical-align-block-elements-expected.html: Added.
+        * fast/css/vertical-align-block-elements.html: Added.
+
 2013-05-10  Alexey Proskuryakov  <ap@apple.com>
 
         <rdar://problem/13666412> Clean up some edge cases of URL parsing.
index fdd1ec2..1e8aca7 100644 (file)
@@ -31,7 +31,6 @@ PASS queryCommandState("subscript") returns false when selecting all of "<sup>he
 PASS queryCommandState("subscript") returns true when selecting all of "<sub>hello</sub>"
 PASS queryCommandState("subscript") returns false when selecting all of "<sub>hello</sub> world"
 PASS queryCommandState("subscript") returns false when selecting all of "hello <sub>world</sub>"
-PASS queryCommandState("subscript") returns true when selecting all of "<div style="vertical-align: sub;">hello world</div>"
 PASS queryCommandState("subscript") returns false when selecting second word of "hello <span style="vertical-align: sub;">world</span> WebKit"
 PASS queryCommandState("superscript") returns false when selecting all of "hello"
 PASS queryCommandState("superscript") returns false when selecting all of "<sub>hello</sub>"
@@ -89,7 +88,6 @@ PASS queryCommandState("subscript") returns false when selecting all of "<sup>he
 PASS queryCommandState("subscript") returns true when selecting all of "<sub>hello</sub>"
 PASS queryCommandState("subscript") returns true when selecting all of "<sub>hello</sub> world"
 PASS queryCommandState("subscript") returns false when selecting all of "hello <sub>world</sub>"
-PASS queryCommandState("subscript") returns true when selecting all of "<div style="vertical-align: sub;">hello world</div>"
 PASS queryCommandState("subscript") returns true when selecting second word of "hello <span style="vertical-align: sub;">world</span> WebKit"
 PASS queryCommandState("superscript") returns false when selecting all of "hello"
 PASS queryCommandState("superscript") returns false when selecting all of "<sub>hello</sub>"
index dc07ea3..350ba16 100644 (file)
@@ -75,7 +75,6 @@ function runTests(editingBehavior) {
     testQueryCommandState("subscript", '<sub>hello</sub>', selectAll, {'mac': true, 'win': true}[editingBehavior]);
     testQueryCommandState("subscript", '<sub>hello</sub> world', selectAll, {'mac': true, 'win': false}[editingBehavior]);
     testQueryCommandState("subscript", 'hello <sub>world</sub>', selectAll, {'mac': false, 'win': false}[editingBehavior]);
-    testQueryCommandState("subscript", '<div style="vertical-align: sub;">hello world</div>', selectAll, {'mac': true, 'win': true}[editingBehavior]);
     testQueryCommandState("subscript", 'hello <span style="vertical-align: sub;">world</span> WebKit', selectSecondWord, {'mac': true, 'win': false}[editingBehavior]);
 
     testQueryCommandState("superscript", 'hello', selectAll, {'mac': false, 'win': false}[editingBehavior]);
diff --git a/LayoutTests/fast/css/vertical-align-block-elements-expected.html b/LayoutTests/fast/css/vertical-align-block-elements-expected.html
new file mode 100644 (file)
index 0000000..7f49bc9
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    div {
+        line-height: 30px;
+        font-size: 14px;
+    }
+    button {
+        height: 30px;
+    }
+</style>
+</script>
+</head>
+<body>
+    <p>Bug: webkit.org/b/111974 - vertical-align only applies to inline-level elements and table-cells. The text below should be aligned horiontally.
+    <div>
+        <button>All </button> This <span>text</span> is <a href="#">aligned</a>.
+    </div>
+</body>
+</html>
+
+
diff --git a/LayoutTests/fast/css/vertical-align-block-elements.html b/LayoutTests/fast/css/vertical-align-block-elements.html
new file mode 100644 (file)
index 0000000..ad6a611
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    div {
+        vertical-align: top;
+        line-height: 20px;
+        height: 20px;
+        font-size: 14px;
+    }
+    button {
+        height: 30px;
+    }
+</style>
+</script>
+</head>
+<body>
+    <p>Bug: webkit.org/b/111974 - vertical-align only applies to inline-level elements and table-cells. The text below should be aligned horiontally.
+    <div>
+        <button>All </button> This <span>text</span> is <a href="#">aligned</a>.
+    </div>
+</body>
+</html>
+
+
index 5dbe9e7..4e3d589 100644 (file)
@@ -1,3 +1,22 @@
+2013-03-17  Robert Hogan  <robert@webkit.org>
+
+        Text flow broken in elements with vertical align top/bottom and inline elements taller than line-height
+        https://bugs.webkit.org/show_bug.cgi?id=111974
+
+        Reviewed by Ryosuke Niwa.
+        
+        Per http://www.w3.org/TR/CSS2/visudet.html#propdef-vertical-align 'vertical-align' only applies to inline and table-cell
+        elements.
+
+        Test: fast/css/vertical-align-block-elements.html
+
+        * rendering/InlineFlowBox.cpp:
+        (WebCore::isTextInBlockElement):
+        (WebCore):
+        (WebCore::InlineFlowBox::adjustMaxAscentAndDescent):
+        (WebCore::InlineFlowBox::computeLogicalBoxHeights):
+        (WebCore::InlineFlowBox::placeBoxesInBlockDirection):
+
 2013-05-11  Benjamin Poulain  <bpoulain@apple.com>
 
         Make CanvasStyle a plain object instead of an RefCounted object
index d301480..30836bd 100644 (file)
@@ -480,6 +480,14 @@ bool InlineFlowBox::requiresIdeographicBaseline(const GlyphOverflowAndFallbackFo
     return false;
 }
 
+static bool verticalAlignApplies(RenderObject* curr)
+{
+    // The vertical-align property only applies to inline elements and table cells.
+    if (curr->isRenderBlock() && !curr->isInline())
+        printf("block-flow block\n");
+    return !curr->isText() || curr->parent()->isInline() || curr->parent()->isTableCell();
+}
+
 void InlineFlowBox::adjustMaxAscentAndDescent(int& maxAscent, int& maxDescent, int maxPositionTop, int maxPositionBottom)
 {
     for (InlineBox* curr = firstChild(); curr; curr = curr->nextOnLine()) {
@@ -487,7 +495,8 @@ void InlineFlowBox::adjustMaxAscentAndDescent(int& maxAscent, int& maxDescent, i
         // positioned elements
         if (curr->renderer()->isOutOfFlowPositioned())
             continue; // Positioned placeholders don't affect calculations.
-        if (curr->verticalAlign() == TOP || curr->verticalAlign() == BOTTOM) {
+
+        if ((curr->verticalAlign() == TOP || curr->verticalAlign() == BOTTOM) && verticalAlignApplies(curr->renderer())) {
             int lineHeight = curr->lineHeight();
             if (curr->verticalAlign() == TOP) {
                 if (maxAscent + maxDescent < lineHeight)
@@ -570,10 +579,10 @@ void InlineFlowBox::computeLogicalBoxHeights(RootInlineBox* rootBox, LayoutUnit&
         rootBox->ascentAndDescentForBox(curr, textBoxDataMap, ascent, descent, affectsAscent, affectsDescent);
 
         LayoutUnit boxHeight = ascent + descent;
-        if (curr->verticalAlign() == TOP) {
+        if (curr->verticalAlign() == TOP && verticalAlignApplies(curr->renderer())) {
             if (maxPositionTop < boxHeight)
                 maxPositionTop = boxHeight;
-        } else if (curr->verticalAlign() == BOTTOM) {
+        } else if (curr->verticalAlign() == BOTTOM && verticalAlignApplies(curr->renderer())) {
             if (maxPositionBottom < boxHeight)
                 maxPositionBottom = boxHeight;
         } else if (!inlineFlowBox || strictMode || inlineFlowBox->hasTextChildren() || (inlineFlowBox->descendantsHaveSameLineHeightAndBaseline() && inlineFlowBox->hasTextDescendants())
@@ -633,9 +642,10 @@ void InlineFlowBox::placeBoxesInBlockDirection(LayoutUnit top, LayoutUnit maxHei
 
         InlineFlowBox* inlineFlowBox = curr->isInlineFlowBox() ? toInlineFlowBox(curr) : 0;
         bool childAffectsTopBottomPos = true;
-        if (curr->verticalAlign() == TOP)
+
+        if (curr->verticalAlign() == TOP && verticalAlignApplies(curr->renderer()))
             curr->setLogicalTop(top);
-        else if (curr->verticalAlign() == BOTTOM)
+        else if (curr->verticalAlign() == BOTTOM && verticalAlignApplies(curr->renderer()))
             curr->setLogicalTop(top + maxHeight - curr->lineHeight());
         else {
             if (!strictMode && inlineFlowBox && !inlineFlowBox->hasTextChildren() && !curr->boxModelObject()->hasInlineDirectionBordersOrPadding()