Bug 13775 - REGRESSION: Popup button text should use "natural" directionality to match the items in the popup menu
Summary: REGRESSION: Popup button text should use "natural" directionality to match th...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
Keywords: InRadar, Regression
Depends on:
Reported: 2007-05-18 07:42 PDT by mitz
Modified: 2007-05-30 12:26 PDT (History)
1 user (show)

See Also:

Test case (192 bytes, text/html)
2007-05-18 07:44 PDT, mitz
no flags Details
Natural directionality patch (1.46 KB, patch)
2007-05-27 07:14 PDT, mitz
no flags Details | Formatted Diff | Diff
Natural directionality patch (59.08 KB, patch)
2007-05-29 12:34 PDT, mitz
no flags Details | Formatted Diff | Diff
Updated patch (59.13 KB, patch)
2007-05-29 13:12 PDT, mitz
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
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.

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.

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