[webkit-reviews] review requested: [Bug 21440] CSS 2.1 failure: bidi-override ignored : [Attachment 86124] patch w/ layout test

[bugzilla-daemon at webkit.org]

Xiaomei Ji <xji at chromium.org> has asked  for review:
Bug 21440: CSS 2.1 failure: bidi-override ignored
https://bugs.webkit.org/show_bug.cgi?id=21440

Attachment 86124: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=86124&action=review

------- Additional Comments from Xiaomei Ji <xji at chromium.org>
I tried to inherit the column properties if he parent style specified it inside
RenderStyle::createAnonymousStyle().
The extra code changes are:


Index: RenderBlock.cpp
===================================================================
--- RenderBlock.cpp	(revision 81184)
+++ RenderBlock.cpp	(working copy)
@@ -239,14 +239,9 @@
     // FIXME: We could save this call when the change only affected
non-inherited properties
     for (RenderObject* child = firstChild(); child; child =
child->nextSibling()) {
	 if (child->isAnonymousBlock()) {
-	     RefPtr<RenderStyle> newStyle = RenderStyle::create();
-	     newStyle->inheritFrom(style());
-	     if (style()->specifiesColumns()) {
-		 if (child->style()->specifiesColumns())
-		     newStyle->inheritColumnPropertiesFrom(style());
-		 if (child->style()->columnSpan())
-		     newStyle->setColumnSpan(true);
-	     }
+	     RefPtr<RenderStyle> newStyle =
RenderStyle::createAnonymousStyle(style());
+	     if (style()->specifiesColumns() && child->style()->columnSpan())
+		 newStyle->setColumnSpan(true);
	     newStyle->setDisplay(BLOCK);
	     child->setStyle(newStyle.release());
	 }
@@ -5873,9 +5866,7 @@

 RenderBlock* RenderBlock::createAnonymousColumnsBlock() const
 {
-    RefPtr<RenderStyle> newStyle = RenderStyle::create();
-    newStyle->inheritFrom(style());
-    newStyle->inheritColumnPropertiesFrom(style());
+    RefPtr<RenderStyle> newStyle = RenderStyle::createAnonymousStyle(style());

     newStyle->setDisplay(BLOCK);

     RenderBlock* newBox = new (renderArena()) RenderBlock(document() /*
anonymous box */);
Index: Source/WebCore/rendering/style/RenderStyle.cpp
===================================================================
--- Source/WebCore/rendering/style/RenderStyle.cpp	(revision 81184)
+++ Source/WebCore/rendering/style/RenderStyle.cpp	(working copy)
@@ -56,6 +56,16 @@
     return adoptRef(new RenderStyle(true));
 }

+PassRefPtr<RenderStyle> RenderStyle::createAnonymousStyle(const RenderStyle*
parentStyle)
+{
+    RefPtr<RenderStyle> newStyle = RenderStyle::create();
+    newStyle->inheritFrom(parentStyle);
+    newStyle->inheritUnicodeBidiFrom(parentStyle);
+    if (parentStyle->specifiesColumns())
+	 newStyle->inheritColumnPropertiesFrom(parentStyle);
+    return newStyle;
+}
+


But that breaks 28 Layout tests under fast/multicol.

I am not sure whether we could inherit column properties by just check whether
parent specifies it.
The 1st original caller of  inheritColumnPropertiesFrom() inside
RenderBlock::styleDidChange() checks both parent and child specifies it.
I do not know the code enough to add it in.