Bug 157672 - [iOS] <select> elements popover should render right-aligned when in RTL mode
Summary: [iOS] <select> elements popover should render right-aligned when in RTL mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-05-13 04:11 PDT by Antoine Quint
Modified: 2016-05-13 19:03 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.70 KB, patch)
2016-05-13 04:14 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2016-05-13 04:11:08 PDT
Now that we correctly render <select> elements when the content is using right-to-left, we also need to propagate this information to the popover shown on iPad for the <select>.
Comment 1 Antoine Quint 2016-05-13 04:11:23 PDT
<rdar://problem/26193442>
Comment 2 Radar WebKit Bug Importer 2016-05-13 04:11:58 PDT
<rdar://problem/26265693>
Comment 3 Antoine Quint 2016-05-13 04:14:43 PDT
Created attachment 278833 [details]
Patch
Comment 4 Darin Adler 2016-05-13 08:46:03 PDT
Comment on attachment 278833 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278833&action=review

Is there a way we can make a regression test for this?

> Source/WebKit2/Shared/AssistedNodeInformation.h:103
> +        , isRTL(false)

In new code, we like to initialize data members where they are defined rather than in the constructor. We should come back and do that here for this struct. I suspect we will be able to remove the constructor.

> Source/WebKit2/UIProcess/ios/forms/WKFormSelectPopover.mm:130
> +    if (writingDirection == UITextWritingDirectionRightToLeft)
> +        self.view.semanticContentAttribute = UISemanticContentAttributeForceRightToLeft;

I’m a little surprised this is needed. Might be nice to add a "why" comment explaining why.
Comment 5 WebKit Commit Bot 2016-05-13 09:05:11 PDT
Comment on attachment 278833 [details]
Patch

Clearing flags on attachment: 278833

Committed r200858: <http://trac.webkit.org/changeset/200858>
Comment 6 WebKit Commit Bot 2016-05-13 09:05:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Dean Jackson 2016-05-13 12:52:40 PDT
Comment on attachment 278833 [details]
Patch

Regarding the regression test: this is something we struggle with on the desktop too. Our test infrastructure there doesn't allow us to screenshot the popup window. I guess we need some way to dump the external window state.
Comment 8 Dean Jackson 2016-05-13 15:51:30 PDT
Comment on attachment 278833 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278833&action=review

>> Source/WebKit2/UIProcess/ios/forms/WKFormSelectPopover.mm:130
>> +        self.view.semanticContentAttribute = UISemanticContentAttributeForceRightToLeft;
> 
> I’m a little surprised this is needed. Might be nice to add a "why" comment explaining why.

Why surprised? I'll add a comment in a followup patch.
Comment 9 Dean Jackson 2016-05-13 18:33:37 PDT
Follow-up in https://bugs.webkit.org/show_bug.cgi?id=157699
Comment 10 Darin Adler 2016-05-13 19:03:07 PDT
Comment on attachment 278833 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278833&action=review

>>> Source/WebKit2/UIProcess/ios/forms/WKFormSelectPopover.mm:130
>>> +        self.view.semanticContentAttribute = UISemanticContentAttributeForceRightToLeft;
>> 
>> I’m a little surprised this is needed. Might be nice to add a "why" comment explaining why.
> 
> Why surprised? I'll add a comment in a followup patch.

I would have expected passing in a string created with stringWithWritingDirection would have sufficed. It surprises me that we also have to separately force right to left.