Summary: | [iOS] <select> elements popover should render right-aligned when in RTL mode | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||
Component: | Forms | Assignee: | Antoine Quint <graouts> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, dino, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Antoine Quint
2016-05-13 04:11:08 PDT
Created attachment 278833 [details]
Patch
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 on attachment 278833 [details] Patch Clearing flags on attachment: 278833 Committed r200858: <http://trac.webkit.org/changeset/200858> All reviewed patches have been landed. Closing bug. 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 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. Follow-up in https://bugs.webkit.org/show_bug.cgi?id=157699 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. |