Bug 13775

Summary: REGRESSION: Popup button text should use "natural" directionality to match the items in the popup menu
Product: WebKit Reporter: mitz
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: adele
Priority: P1 Keywords: InRadar, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Test case
none
Natural directionality patch
none
Natural directionality patch
none
Updated patch darin: review+

Description mitz 2007-05-18 07:42:23 PDT
In the attached test case, there's a popup with two items. In the menu that pops up, both items have the word "English" on the left and the Hebrew word on the right. However, in TOT, when the second item is selected, the popup button shows the words in reverse order: Hebrew on the left and English on the right.

The problem is that AppKit uses "natural" writing direction for the popup menu items, roughly meaning that the first character with strong directionality determines the paragraph directionality to use. CSS (and WebKit) do not have a notion of "direction: natural", so everything has the same direction (LTR now, the <select>'s direction prior to <http://trac.webkit.org/projects/webkit/changeset/21541>).

I think it may be possible for the popup button to fake "natural" directionality for this specific case.
Comment 1 mitz 2007-05-18 07:44:16 PDT
Created attachment 14607 [details]
Test case
Comment 2 Adele Peterson 2007-05-18 11:22:00 PDT
<rdar://problem/5213488>
Comment 3 mitz 2007-05-27 07:14:15 PDT
Created attachment 14748 [details]
Natural directionality patch

This is a patch that works as far as I can tell, but I think that both the left alignment and the "natural" directionality are very Mac-specific quirks that have to do with the Mac popup implementation, so it doesn't feel right to do them in cross-platform code. Those style adjustments are really something that the popup dictates, so it might make sense to do them in a PopupMenu static method (since you don't want to create a popup just for that) or a RenderTheme method.
Comment 4 Dave Hyatt 2007-05-27 11:31:08 PDT
Or you could do this in cross-platform code but have a PopupMenu method that asks whether or not you should do it.
Comment 5 mitz 2007-05-29 12:34:26 PDT
Created attachment 14770 [details]
Natural directionality patch
Comment 6 Darin Adler 2007-05-29 12:44:58 PDT
Comment on attachment 14770 [details]
Natural directionality patch

I think we should refactor the loop that figures out the TextDirection of a run of text into a helper function.

r=me
Comment 7 mitz 2007-05-29 13:12:23 PDT
Created attachment 14771 [details]
Updated patch

Factored out the loop.
Comment 8 Darin Adler 2007-05-30 07:36:47 PDT
Comment on attachment 14771 [details]
Updated patch

+TextDirection textDirectionForParagraph(StringImpl* paragraph)

Should be marked "static" so it gets internal linkage.

r=me
Comment 9 Mark Rowe (bdash) 2007-05-30 12:26:34 PDT
Landed in r21900 with "static" added as requested.