The video controls should flip the volume control layout when the system is set to use a right-to-left language.
<rdar://problem/25971769>
Created attachment 281503 [details] Patch
Created attachment 281507 [details] Patch
Seems like this should respect the userInterfaceDirectionPolicy property of the configuration. We should also use booleans with “LTR” rather than “RTL” in the name (and set their values appropriately).
(In reply to comment #4) > Seems like this should respect the userInterfaceDirectionPolicy property of > the configuration. This change only applies to full-screen video controls and the directionality should be dictated by the WebView and not the content, so the policy should not apply. > We should also use booleans with “LTR” rather than “RTL” in the name (and > set their values appropriately). thorton provided feedback on IRC that we should propagate the direction rather than an RTL flag, so I will make that change.
Created attachment 281532 [details] Patch
Attachment 281532 [details] did not pass style-queue: ERROR: Source/WebCore/page/Page.h:142: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/ChangeLog:21: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
I'll have a test up in the morning.
Created attachment 281568 [details] Patch
Attachment 281568 [details] did not pass style-queue: ERROR: Source/WebCore/page/Page.h:142: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/testing/Internals.h:489: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 2 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 281568 [details] Patch The WebCore and LayoutTest changes look good to me; this patch should get a WK2 Owner to sign off on the WK2 changes.
Comment on attachment 281568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281568&action=review > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:2242 > + this.controls.volumeBox.classList.toggle(this.ClassNames.usesRTLUserInterfaceLayoutDirection, usesRTLUserInterfaceLayoutDirection); You seem to have not followed mitz's review comment here. This should be about LTR, not RTL. > Source/WebCore/page/Page.h:142 > + enum class UserInterfaceLayoutDirection { LTR, RTL }; Really, right here in Page? You're not in good company, so it seems like a bad place :) > Source/WebCore/testing/Internals.cpp:3352 > + page->setUserInterfaceLayoutDirection(userInterfaceLayoutDirection == UserInterfaceLayoutDirection::LTR ? Page::UserInterfaceLayoutDirection::LTR : Page::UserInterfaceLayoutDirection::RTL); :| (fix the below, and this will go away) > Source/WebCore/testing/Internals.h:489 > + enum class UserInterfaceLayoutDirection { LTR, RTL }; Why so many of these? Why not just one, in its own file. > Source/WebKit2/UIProcess/UserInterfaceLayoutDirection.h:-31 > -enum class UserInterfaceLayoutDirection { LTR, RTL }; It was already in its own file!
Created attachment 281580 [details] Patch
Attachment 281580 [details] did not pass style-queue: ERROR: Source/WebCore/platform/UserInterfaceLayoutDirection.h:31: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/testing/Internals.h:489: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 281580 [details] Patch Attachment 281580 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1519761 New failing tests: http/tests/media/hls/video-controls-live-stream.html
Created attachment 281588 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281580 [details] Patch Attachment 281580 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1519764 New failing tests: http/tests/media/hls/video-controls-live-stream.html
Created attachment 281590 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 281580 [details] Patch Attachment 281580 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1519798 New failing tests: http/tests/media/hls/video-controls-live-stream.html
Created attachment 281591 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 281592 [details] Patch for landing
Attachment 281592 [details] did not pass style-queue: ERROR: Source/WebCore/platform/UserInterfaceLayoutDirection.h:31: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/testing/Internals.h:489: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 281592 [details] Patch for landing Clearing flags on attachment: 281592 Committed r202183: <http://trac.webkit.org/changeset/202183>
All reviewed patches have been landed. Closing bug.