Bug 157672 – [iOS] <select> elements popover should render right-aligned when in RTL mode

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Antoine Quint]

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

[Antoine Quint]

<rdar://problem/26193442>

[Radar WebKit Bug Importer]

<rdar://problem/26265693>

[Antoine Quint]

Created attachment 278833 [details]
Patch

[Darin Adler]

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.

[WebKit Commit Bot]

Comment on attachment 278833 [details]
Patch

Clearing flags on attachment: 278833

Committed r200858: <http://trac.webkit.org/changeset/200858>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dean Jackson]

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.

[Dean Jackson]

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.

[Dean Jackson]

Follow-up in https://bugs.webkit.org/show_bug.cgi?id=157699

[Darin Adler]

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.