Bug 50969

Safari 5.0.3 handles this correctly, but Chromium with TOT WebKit is still broken. Haven't nailed down why precisely yet.

Levi: perhaps what you're seeing is Cocoa aligning Hebrew text to the right and English text to the left? What about the following case: <select> <option dir="rtl">Right aligned</option> <option dir="ltr">left aligned</option> <option dir="ltr">מיושר לשמאל</option> <option dir="rtl">מיושר לימין</option> </select

That'd be it, thanks! Glancing at the interface I didn't see how that was getting communicated... :)

Refer to http://trac.webkit.org/changeset/76983 for detail. Other ports need to 1. define Chrome::selectItemAlignmentFollowsMenuWritingDirection() as true. 2. implement rendering of <option> in dropdown using <option>'s dir as what mitz did in webkit2.

Created attachment 81072 [details] patch for Chromium port

Comment on attachment 81072 [details] patch for Chromium port View in context: https://bugs.webkit.org/attachment.cgi?id=81072&action=review I think you can address my comments above and commit this. Thanks! > Source/WebCore/ChangeLog:5 > + Implement "<option> should support the dir attribute and be displayed accordingly both in the drop-down and after being chosen" for chromium port after r76983. This is pretty verbose. "<option> should implement the dir attribute." sums it up. (Just exposing the attribute isn't implementing it. The code needs to display the text correctly, etc.) > Source/WebCore/ChangeLog:12 > + Turn on it for <select>. This is really long. I think you can leave it out entirely. You explained it well in the file/function level comments. > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:84 > + false // restrictWidthOfListBox For future reference, misc formatting fixes mixed in a fix on lines where you aren't doing changes is frowned on because it obscures the change being done, but ok. > Source/WebKit/chromium/ChangeLog:13 > + Same comments as before.

Committed r77654: <http://trac.webkit.org/changeset/77654> for chromium port.

Please resolve bugs after landing them :)

(In reply to comment #8) > Please resolve bugs after landing them :) I think it's still open because it isn't fixed for all ports.

Comment on attachment 81072 [details] patch for Chromium port clear flags for landed patch.

keep it open for other ports.

Created attachment 81512 [details] Patch for Chromium on Mac

Adding Darin due to modification in Source/WebKit/chromium/public/WebMenuItemInfo.h.

Comment on attachment 81512 [details] Patch for Chromium on Mac View in context: https://bugs.webkit.org/attachment.cgi?id=81512&action=review > Source/WebKit/chromium/public/WebMenuItemInfo.h:61 > + bool directionalOverride; it is not clear what directionalOverride means. would it perhaps be better named hasTextDirectionOverride as that is what WebCore seems to be using for this field already? if i'm the embedder, can i'm trying to render a menu based on this information, what am i supposed to do with "directionalOverride" and "textDirection"? I think the other fields are fairly self-explanatory, but these fields are not similarly obvious.

Created attachment 81517 [details] Patch for Chromium on Mac, v2 Re directionalOverride, agreed; changed. Re textDirection. This is of enum type, which IMO makes it clear. platform/PopupMenuStyle.h has textDirection() returning the same enum. "Direction" always means LTR/RTL when it comes to text. See editing/WritingDirection.h, editing/EditingStyle.h's textDirection(), WK/chromium/public/WebTextDirection.h, etc.

A summary of the changes in case I made Csaba and Martin confused in IRC. 1. This bug is about in what directionality <select>/<option> should be rendered. It is standardized in HTML5. Basically, the items should be displayed according to its directionality defined in <option> in both button text (when chosen) and in drop-down menu. As to how <select>/<option> should be aligned, it seems not defined in standard. Webkit currently alignes item in drop-down box and in button text (when chosen) according to <select>'s directionality. It disregards text-align attribute in <select>. Please refer to mitz's comment in https://bugs.webkit.org/show_bug.cgi?id=19785#c19. 2. Mitz implemented rendering button text according to its directionality defined in <option> and aligning button text based on <select>'s directionality in r76983. The implementation is exposed through ChromeClient::selectItemAlignmentFollowsMenuWritingDirection(). In the same changset, Mitz also implemented rending the items in drop-down box using its directionality defined in <option> and aligning items in drop-down box using <select>'s directionality for Mac OS X Snow leopard. 3. Other ports need to do the following to achieve the same behavior: 3.1 override selectItemAlignmentFollowsMenuWritingDirection() as true and override selectItemWritingDirectionIsNatural() as false so that button text is rendered correctly on both directionality and alignment. 3.2 implement rendering of items in drop-down box using <option>'s directionality and aligning by <select>'s writing direction as what mitz did in webkit2 or platform/mac/PopupMenuMac.mm. Chromium already implemented aligning items in drop-down box using <select>'s writing direction in WebCore/platform/chromium/PopupMenuChromium.cpp a while back. It also has implementation ready to render items in drop-down using item's directionality, and it just needs to turn on the switch. So, the chromium's patch for Windows and Linux committed here basically do not have any implementation of 3.2. 4. I added fast/text/international/pop-up-button-text-alignment-and-direction, which *only* test the rendering of button text. It's skipped in gtk and qt for now. ossy and mrobinson will take care of qt and gtk ports. To check the correctness, the button text should be rendered the same as the text below it per directionality and alignment as specified in the 1st line in the test. 5. We should add automatic test for drop-down box as well.

(In reply to comment #16) > 3.2 implement rendering of items in drop-down box using <option>'s directionality and aligning by <select>'s writing direction as what mitz did in webkit2 or platform/mac/PopupMenuMac.mm. The patch waiting for review here implements half of this for Chromium Mac. I have the other half (the Chromium side) ready for commit (http://codereview.chromium.org/6410125/). When landed, this will give Chromium Mac the exact same behavior as what mitz did for PopupMenuMac.mm. (Ping; please re-review my patch attached here.)

(In reply to comment #15) > Created an attachment (id=81517) [details] > Patch for Chromium on Mac, v2 > > Re directionalOverride, agreed; changed. > > Re textDirection. This is of enum type, which IMO makes it clear. platform/PopupMenuStyle.h has textDirection() returning the same enum. "Direction" always means LTR/RTL when it comes to text. See editing/WritingDirection.h, editing/EditingStyle.h's textDirection(), WK/chromium/public/WebTextDirection.h, etc. Can you use WebTextDirectionDefault in place of hasTextDirectionOverride? Unlike WebCore::TextDirection, WebTextDirection has the "unspecified" state. It seems like hasTextDirectionOverride is therefore unnecessary / redundant.

(In reply to comment #18) > Can you use WebTextDirectionDefault in place of hasTextDirectionOverride? > > Unlike WebCore::TextDirection, WebTextDirection has the "unspecified" state. It seems like hasTextDirectionOverride is therefore unnecessary / redundant. No. An override is orthogonal to the directionality. Please look at http://trac.webkit.org/browser/trunk/Source/WebCore/manual-tests/pop-up-alignment-and-direction.html , particularly options #3 and #4 in the popup. The fact there is an override is a separate fact from LTR/RTL.

Comment on attachment 81517 [details] Patch for Chromium on Mac, v2 Clearing flags on attachment: 81517 Committed r78193: <http://trac.webkit.org/changeset/78193>

(In reply to comment #16) > 3. Other ports need to do the following to achieve the same behavior: > 3.1 override selectItemAlignmentFollowsMenuWritingDirection() as true [...] > 3.2 implement rendering of items in drop-down box [...] I just landed r74622 in Chromium which takes advantage of the plumbed data to properly render items in the drop-down box. Chromium Mac is done. Leaving this bug open; there are some more platforms to fix.

This added two new POD members, but didn't add initializations to the constructor. Please fix that, since it causes clients to introduce uninitialized memory in normal use.

(Added to WebMenuItemInfo, sorry)

Created attachment 85416 [details] Forgot to initialize variables

Comment on attachment 85416 [details] Forgot to initialize variables Clearing flags on attachment: 85416 Committed r80862: <http://trac.webkit.org/changeset/80862>

I have verified that it is fixed on Windows and Linux. Yay!

Was this bug specific to Chromium port or no?

(In reply to comment #27) > Was this bug specific to Chromium port or no? According to comments 9 and 11, no, this is an issue that all ports have to handle.

(In reply to Jeremy Moskovich from comment #2) > Levi: perhaps what you're seeing is Cocoa aligning Hebrew text to the right > and English text to the left? What about the following case: > > <select> > <option dir="rtl">Right aligned</option> > <option dir="ltr">left aligned</option> > <option dir="ltr">מיושר לשמאל</option> > <option dir="rtl">מיושר לימין</option> > </select I changed it into JSFiddle: Link - https://jsfiddle.net/9rny41Lh/ and I am able to reproduce this in Safari Technology Preview 155 and it does not show options aligned left to right similar to Chrome Canary 108 and Firefox Nightly 107. NOTE - Chrome show all options in drop-down as LTR while when selecting, the RTL options are displayed as RTL. In case of Firefox, the options are displayed in drop-down as appropriate as well and "Tick" also respect the LTR and RTL. I would say "Firefox" has superior implementation IMO. _____ Just to share updated testing results. Thanks!