Bug 50969 – <option> should support the dir attribute and be displayed accordingly both in the dropdown and after being chosen

• Component: DOM
• Status: REOPENED

[Xiaomei Ji]

Following is from the spec:

10.6.4 Text rendered in native user interfaces

User agents are expected to honor the Unicode semantics of text that is exposed in user interfaces, for example supporting the bidirectional algorithm in text shown in dialogs, title bars, pop-up menus, and tooltips. Text from elements (either attribute values or the contents of elements) is expected to be rendered in a manner that honors the directionality of the element from which the text was obtained.

Consider the following markup, which has Hebrew text asking for a programming language, the languages being text for which a left-to-right direction is important given the punctuation in some of their names:

<p dir="rtl" lang="he">
 <label>
  בחר שפת תכנות:
  <select>
   <option dir="ltr">C++</option>
   <option dir="ltr">C#</option>
   <option dir="ltr">FreePascal</option>
   <option dir="ltr">F#</option>
  </select>
 </label>
</p>
If the select element was rendered as a drop down box, a correct rendering would ensure that the punctuation was the same both in the drop down, and in the box showing the current selection.

Note: with image showing the item showing in drop-down and selection are both in LTR directionality and right alignment.

[Levi Weintraub]

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

[Jeremy Moskovich]

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

[Levi Weintraub]

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

[Xiaomei Ji]

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.

[Xiaomei Ji]

Created attachment 81072 [details]
patch for Chromium port

[David Levin]

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.

[Xiaomei Ji]

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

[David Levin]

Please resolve bugs after landing them :)

[Levi Weintraub]

(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.

[Xiaomei Ji]

Comment on attachment 81072 [details]
patch for Chromium port

clear flags for landed patch.

[Xiaomei Ji]

keep it open for other ports.

[Avi Drissman]

Created attachment 81512 [details]
Patch for Chromium on Mac

[David Levin]

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

[Darin Fisher (:fishd, Google)]

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.

[Avi Drissman]

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.

[Xiaomei Ji]

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.

[Avi Drissman]

(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.)

[Darin Fisher (:fishd, Google)]

(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.

[Avi Drissman]

(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.

[WebKit Commit Bot]

Comment on attachment 81517 [details]
Patch for Chromium on Mac, v2

Clearing flags on attachment: 81517

Committed r78193: <http://trac.webkit.org/changeset/78193>

[Avi Drissman]

(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.

[Stuart Morgan]

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.

[Stuart Morgan]

(Added to WebMenuItemInfo, sorry)

[Avi Drissman]

Created attachment 85416 [details]
Forgot to initialize variables

[WebKit Commit Bot]

Comment on attachment 85416 [details]
Forgot to initialize variables

Clearing flags on attachment: 85416

Committed r80862: <http://trac.webkit.org/changeset/80862>

[Aharon (Vladimir) Lanin]

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

[Ryosuke Niwa]

Was this bug specific to Chromium port or no?

[Avi Drissman]

(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.

[Ahmad Saleem]

(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!