Title: [157437] trunk
Revision
157437
Author
[email protected]
Date
2013-10-14 18:06:20 -0700 (Mon, 14 Oct 2013)

Log Message

Windows select element doesn't draw RTL properly.
https://bugs.webkit.org/show_bug.cgi?id=122785.

Reviewed by Brent Fulgham.

Problems include the popup items not drawing on the right hand side and 
not respecting the direction or the directional override styling of the option.
The selected element (drawn in the actual select element) also doesn't respect 
the style settings of the selected menu option.

Tests:
Covered by fast/text/international/pop-up-button-text-alignment-and-direction.html.

* platform/win/PopupMenuWin.cpp:
(WebCore::PopupMenuWin::paint):
* WebCoreSupport/WebChromeClient.cpp:
(WebChromeClient::selectItemWritingDirectionIsNatural):
(WebChromeClient::selectItemAlignmentFollowsMenuWritingDirection):
* platform/win/fast/text/international/pop-up-button-text-alignment-and-direction-expected.txt: Removed.

Modified Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (157436 => 157437)


--- trunk/LayoutTests/ChangeLog	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/LayoutTests/ChangeLog	2013-10-15 01:06:20 UTC (rev 157437)
@@ -1,3 +1,12 @@
+2013-10-14  Roger Fong  <[email protected]>
+
+        Windows select element doesn't draw RTL properly.
+        https://bugs.webkit.org/show_bug.cgi?id=122785.
+
+        Reviewed by Brent Fulgham.
+
+        * platform/win/fast/text/international/pop-up-button-text-alignment-and-direction-expected.txt: Removed.
+
 2013-10-14  Ryosuke Niwa  <[email protected]>
 
         Crash in WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>::createBidiRunsForLine

Deleted: trunk/LayoutTests/platform/win/fast/text/international/pop-up-button-text-alignment-and-direction-expected.txt (157436 => 157437)


--- trunk/LayoutTests/platform/win/fast/text/international/pop-up-button-text-alignment-and-direction-expected.txt	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/LayoutTests/platform/win/fast/text/international/pop-up-button-text-alignment-and-direction-expected.txt	2013-10-15 01:06:20 UTC (rev 157437)
@@ -1,115 +0,0 @@
-layer at (0,0) size 800x600
-  RenderView at (0,0) size 800x600
-layer at (0,0) size 800x544
-  RenderBlock {HTML} at (0,0) size 800x544
-    RenderBody {BODY} at (8,16) size 784x520
-      RenderBlock {P} at (0,0) size 784x18
-        RenderText {#text} at (0,0) size 695x18
-          text run at (0,0) width 486: "Verify that the alignment and writing direction of each selected item matches "
-          text run at (486,0) width 209: "the one below the pop-up button."
-      RenderBlock {DIV} at (0,34) size 784x242
-        RenderMenuList {SELECT} at (0,0) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,23) size 470x36
-          RenderText {#text} at (10,10) size 163x16
-            text run at (10,10) width 32: "First "
-            text run at (41,10) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-            text run at (89,10) width 17: "03"
-            text run at (105,10) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-            text run at (141,10) width 32: " fifth"
-        RenderMenuList {SELECT} at (0,61) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,84) size 470x36
-          RenderText {#text} at (10,10) size 163x16
-            text run at (10,10) width 27: "fifth"
-            text run at (36,10) width 52 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} "
-            text run at (87,10) width 18: "03"
-            text run at (104,10) width 41 RTL: " \x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-            text run at (144,10) width 29: "First"
-        RenderMenuList {SELECT} at (0,122) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,145) size 470x36
-          RenderText {#text} at (10,10) size 163x16
-            text run at (10,10) width 163 LTR override: "First \x{5E9}\x{5E0}\x{5D9}\x{5D4} (03) \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} fifth"
-        RenderMenuList {SELECT} at (0,183) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,206) size 470x36
-          RenderText {#text} at (10,10) size 163x16
-            text run at (10,10) width 163 RTL override: "First \x{5E9}\x{5E0}\x{5D9}\x{5D4} (03) \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} fifth"
-      RenderBlock {DIV} at (0,278) size 784x242
-        RenderMenuList {SELECT} at (0,0) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,23) size 470x36
-          RenderText {#text} at (297,10) size 163x16
-            text run at (297,10) width 33: "First "
-            text run at (329,10) width 48 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-            text run at (376,10) width 18: "03"
-            text run at (393,10) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-            text run at (429,10) width 31: " fifth"
-        RenderMenuList {SELECT} at (0,61) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,84) size 470x36
-          RenderText {#text} at (297,10) size 163x16
-            text run at (297,10) width 28: "fifth"
-            text run at (324,10) width 52 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} "
-            text run at (375,10) width 17: "03"
-            text run at (391,10) width 42 RTL: " \x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-            text run at (432,10) width 28: "First"
-        RenderMenuList {SELECT} at (0,122) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,145) size 470x36
-          RenderText {#text} at (297,10) size 163x16
-            text run at (297,10) width 163 LTR override: "First \x{5E9}\x{5E0}\x{5D9}\x{5D4} (03) \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} fifth"
-        RenderMenuList {SELECT} at (0,183) size 500x21 [bgcolor=#FFFFFF]
-          RenderBlock (anonymous) at (0,0) size 500x21
-            RenderText at (8,2) size 163x16
-              text run at (8,2) width 32: "First "
-              text run at (39,2) width 49 RTL: ") \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA}"
-              text run at (87,2) width 17: "03"
-              text run at (103,2) width 37 RTL: "\x{5E9}\x{5E0}\x{5D9}\x{5D4} ("
-              text run at (139,2) width 32: " fifth"
-        RenderBlock {DIV} at (0,206) size 470x36
-          RenderText {#text} at (297,10) size 163x16
-            text run at (297,10) width 163 RTL override: "First \x{5E9}\x{5E0}\x{5D9}\x{5D4} (03) \x{5E8}\x{5D1}\x{5D9}\x{5E2}\x{5D9}\x{5EA} fifth"

Modified: trunk/Source/WebCore/ChangeLog (157436 => 157437)


--- trunk/Source/WebCore/ChangeLog	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/Source/WebCore/ChangeLog	2013-10-15 01:06:20 UTC (rev 157437)
@@ -36,6 +36,23 @@
 
 2013-10-14  Roger Fong  <[email protected]>
 
+        Windows select element doesn't draw RTL properly.
+        https://bugs.webkit.org/show_bug.cgi?id=122785.
+
+        Reviewed by Brent Fulgham.
+
+        Covered by fast/text/international/pop-up-button-text-alignment-and-direction.html.
+
+        Problems include the popup items not drawing on the right hand side and 
+        not respecting the direction or the directional override styling of the option.
+        The selected element (drawn in the actual select element) also doesn't respect 
+        the style settings of the selected menu option.
+
+        * platform/win/PopupMenuWin.cpp:
+        (WebCore::PopupMenuWin::paint):
+
+2013-10-14  Roger Fong  <[email protected]>
+
         [Windows] Unreviewed build fix.
 
         * WebCore.vcxproj/WebCoreCommon.props:

Modified: trunk/Source/WebCore/platform/win/PopupMenuWin.cpp (157436 => 157437)


--- trunk/Source/WebCore/platform/win/PopupMenuWin.cpp	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/Source/WebCore/platform/win/PopupMenuWin.cpp	2013-10-15 01:06:20 UTC (rev 157437)
@@ -649,10 +649,8 @@
         }
 
         String itemText = client()->itemText(index);
-            
-        TextDirection direction = (itemText.defaultWritingDirection() == U_RIGHT_TO_LEFT) ? RTL : LTR;
-        TextRun textRun(itemText, 0, 0, TextRun::AllowTrailingExpansion, direction);
 
+        TextRun textRun(itemText, 0, 0, TextRun::AllowTrailingExpansion, itemStyle.textDirection(), itemStyle.hasTextDirectionOverride());
         context.setFillColor(optionTextColor, ColorSpaceDeviceRGB);
         
         Font itemFont = client()->menuStyle().font();
@@ -665,9 +663,17 @@
         
         // Draw the item text
         if (itemStyle.isVisible()) {
-            int textX = max<int>(0, client()->clientPaddingLeft() - client()->clientInsetLeft());
-            if (RenderTheme::defaultTheme()->popupOptionSupportsTextIndent() && itemStyle.textDirection() == LTR)
-                textX += minimumIntValueForLength(itemStyle.textIndent(), itemRect.width());
+            int textX = 0;
+            if (client()->menuStyle().textDirection() == LTR) {
+                textX = max<int>(0, client()->clientPaddingLeft() - client()->clientInsetLeft());
+                if (RenderTheme::defaultTheme()->popupOptionSupportsTextIndent())
+                    textX += minimumIntValueForLength(itemStyle.textIndent(), itemRect.width());
+            } else {
+                textX = itemRect.width() - client()->menuStyle().font().width(textRun);
+                textX = min<int>(textX, textX - client()->clientPaddingRight() + client()->clientInsetRight());
+                if (RenderTheme::defaultTheme()->popupOptionSupportsTextIndent())
+                    textX -= minimumIntValueForLength(itemStyle.textIndent(), itemRect.width());
+            }
             int textY = itemRect.y() + itemFont.fontMetrics().ascent() + (itemRect.height() - itemFont.fontMetrics().height()) / 2;
             context.drawBidiText(itemFont, textRun, IntPoint(textX, textY));
         }

Modified: trunk/Source/WebKit/win/ChangeLog (157436 => 157437)


--- trunk/Source/WebKit/win/ChangeLog	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/Source/WebKit/win/ChangeLog	2013-10-15 01:06:20 UTC (rev 157437)
@@ -1,3 +1,14 @@
+2013-10-14  Roger Fong  <[email protected]>
+
+        Windows select element doesn't draw RTL properly.
+        https://bugs.webkit.org/show_bug.cgi?id=122785.
+
+        Reviewed by Brent Fulgham.
+
+        * WebCoreSupport/WebChromeClient.cpp:
+        (WebChromeClient::selectItemWritingDirectionIsNatural):
+        (WebChromeClient::selectItemAlignmentFollowsMenuWritingDirection):
+
 2013-10-14  Tim Horton  <[email protected]>
 
         Virtualize PlatformCALayer

Modified: trunk/Source/WebKit/win/WebCoreSupport/WebChromeClient.cpp (157436 => 157437)


--- trunk/Source/WebKit/win/WebCoreSupport/WebChromeClient.cpp	2013-10-15 00:38:10 UTC (rev 157436)
+++ trunk/Source/WebKit/win/WebCoreSupport/WebChromeClient.cpp	2013-10-15 01:06:20 UTC (rev 157437)
@@ -793,12 +793,12 @@
 
 bool WebChromeClient::selectItemWritingDirectionIsNatural()
 {
-    return true;
+    return false;
 }
 
 bool WebChromeClient::selectItemAlignmentFollowsMenuWritingDirection()
 {
-    return false;
+    return true;
 }
 
 bool WebChromeClient::hasOpenedPopup() const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to