| Summary: | SuggestionPicker should support rtl | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Keishi Hattori <keishi> | ||||||||||
| Component: | Forms | Assignee: | Keishi Hattori <keishi> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | tkent, webkit.review.bot | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Keishi Hattori
2012-09-25 05:02:06 PDT
Created attachment 165602 [details]
Patch
Comment on attachment 165602 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165602&action=review > LayoutTests/ChangeLog:9 > + * platform/chromium-mac/platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl-expected.png: Added. Does this result match to your expectation? The content of the popup looks not RTL, and looks same with date-suggestion-picker-appearance-expected.png in Bug 97551. Created attachment 165927 [details]
Patch
Comment on attachment 165927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165927&action=review > Source/WebCore/platform/DateTimeChooser.h:50 > + bool isValueRTL; "value" sounds this represents the direction of each of <option> values. isAnchorElementRTL ? > LayoutTests/ChangeLog:10 > + * platform/chromium-mac/platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl-expected.png: Added. The content of the popup still looks LTR. Created attachment 165933 [details]
Patch
(In reply to comment #5) > Created an attachment (id=165933) [details] > Patch What's wrong with the previous patch? Comment on attachment 165933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165933&action=review > Source/WebCore/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). OOPS! > LayoutTests/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). OOPS! Created attachment 165938 [details]
Patch
(In reply to comment #6) > (In reply to comment #5) > > Created an attachment (id=165933) [details] [details] > > Patch > > What's wrong with the previous patch? I renamed to isAnchorElementRTL. And I changed the test to set rtl to the body because that is more common and we can align popup position to the right in the future. (In reply to comment #9) > I renamed to isAnchorElementRTL. And I changed the test to set rtl to the body because that is more common and we can align popup position to the right in the future. They shouldn't affect the direction of the popup content. Why was it corrected? (In reply to comment #10) > (In reply to comment #9) > > I renamed to isAnchorElementRTL. And I changed the test to set rtl to the body because that is more common and we can align popup position to the right in the future. > > They shouldn't affect the direction of the popup content. Why was it corrected? I forgot to reset the expected image in the previous patch(165927). (In reply to comment #11) > I forgot to reset the expected image in the previous patch(165927). I see. I recommend you explain difference from the previous patch when you post a new patch. Silent update confuses reviewers. Comment on attachment 165938 [details]
Patch
ok
Comment on attachment 165938 [details] Patch Clearing flags on attachment: 165938 Committed r129738: <http://trac.webkit.org/changeset/129738> All reviewed patches have been landed. Closing bug. |