Compute the correct overflow-x and overflow-y values for table elements

Source/core/css/resolver/StyleAdjuster.cpp

Index: Source/core/css/resolver/StyleAdjuster.cpp
diff --git a/Source/core/css/resolver/StyleAdjuster.cpp b/Source/core/css/resolver/StyleAdjuster.cpp
index ea0adfd5adacb3065e46e9a919226b7510e37cc5..4e0949ec078679c936c8fe29bd222c675a76ad8c 100644
--- a/Source/core/css/resolver/StyleAdjuster.cpp
+++ b/Source/core/css/resolver/StyleAdjuster.cpp
@@ -425,13 +425,33 @@ void StyleAdjuster::adjustStyleForHTMLElement(ComputedStyle& style, const Comput
         return;
     }
 }
+inline bool overflowValueAllowedOnTable(EOverflow overflow)
+{
+    return overflow == OSCROLL || overflow == OAUTO || overflow == OOVERLAY || overflow == OPAGEDX || overflow == OPAGEDY;

[mstensho (USE GERRIT), 2015/07/27 09:33:30]

None of these values are allowed on tables (and the function is called
overflowValueAllowedOnTable()). These are the very values that are disallowed.

I don't think listing the disallowed values makes any sense. It's not
future-proof.

[rhogan, 2015/07/27 18:18:55]

Haha, just testing! ;)
I agree and have switched it back.

[Julien - ping for review, 2015/07/27 20:08:34]

Looks like I am overruled on that.

/me goes back to play with glue in a corner and let's Morten handle this review

+}
 
 void StyleAdjuster::adjustOverflow(ComputedStyle& style)
 {
     ASSERT(style.overflowX() != OVISIBLE || style.overflowY() != OVISIBLE);
 
-    // If either overflow value is not visible, change to auto.
-    if (style.overflowX() == OVISIBLE && style.overflowY() != OVISIBLE) {
+    if (style.display() == TABLE || style.display() == INLINE_TABLE) {
+        // Tables only support overflow:hidden and overflow:visible and ignore anything else,
+        // see http://dev.w3.org/csswg/css2/visufx.html#overflow. As a table is not a block
+        // container box the rules for resolving conflicting x and y values in CSS Overflow Module
+        // Level 3 do not apply. Arguably overflow-x and overflow-y aren't allowed on tables but
+        // all UAs allow it.
+        if (overflowValueAllowedOnTable(style.overflowX()))
+            style.setOverflowX(OVISIBLE);
+        if (overflowValueAllowedOnTable(style.overflowY()))
+            style.setOverflowY(OVISIBLE);
+        // If we are left with conflicting overflow values for the x and y axes on a table then resolve
+        // both to OVISIBLE. This is interoperable behaviour but is not specced anywhere.
+        if (style.overflowX() == OVISIBLE && style.overflowY() != OVISIBLE)

[mstensho (USE GERRIT), 2015/07/27 09:33:30]

No need for the style.overflowY() != OVISIBLE condition.

[rhogan, 2015/07/27 18:18:55]

Right enough!

+            style.setOverflowY(OVISIBLE);
+        else if (style.overflowY() == OVISIBLE && style.overflowX() != OVISIBLE)

[mstensho (USE GERRIT), 2015/07/27 09:33:30]

ditto.

+            style.setOverflowX(OVISIBLE);
+    } else if (style.overflowX() == OVISIBLE && style.overflowY() != OVISIBLE) {
+        // If either overflow value is not visible, change to auto.
         // FIXME: Once we implement pagination controls, overflow-x should default to hidden
         // if overflow-y is set to -webkit-paged-x or -webkit-page-y. For now, we'll let it
         // default to auto so we can at least scroll through the pages.
@@ -440,16 +460,6 @@ void StyleAdjuster::adjustOverflow(ComputedStyle& style)
         style.setOverflowY(OAUTO);
     }
 
-    // Table rows, sections and the table itself will support overflow:hidden and will ignore scroll/auto.
-    // FIXME: Eventually table sections will support auto and scroll.
-    if (style.display() == TABLE || style.display() == INLINE_TABLE
-        || style.display() == TABLE_ROW_GROUP || style.display() == TABLE_ROW) {
-        if (style.overflowX() != OVISIBLE && style.overflowX() != OHIDDEN)
-            style.setOverflowX(OVISIBLE);
-        if (style.overflowY() != OVISIBLE && style.overflowY() != OHIDDEN)
-            style.setOverflowY(OVISIBLE);
-    }
-
     // Menulists should have visible overflow
     if (style.appearance() == MenulistPart) {
         style.setOverflowX(OVISIBLE);