Top controls bars should invert with right-to-left user interface layout direction locale

• Component: Media
• Status: RESOLVED FIXED

[Antoine Quint]

When we're displaying media controls in an RTL locale, we should:

- invert the two top controls bars
- invert the layout order for the fullscreen / PiP controls bar
- orient the volume button the opposite direction when presented in a top controls bar

[Antoine Quint]

<rdar://problem/32863552>

[Antoine Quint]

Created attachment 314161 [details]
Patch

[Dean Jackson]

Comment on attachment 314161 [details]
Patch

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

> Source/WebCore/ChangeLog:17
> +        * Modules/modern-media-controls/controls/icon-service.js: Add new RTL variants for the mute and unmute icons.

Why didn't we just flip the existing ones?

> Source/WebCore/ChangeLog:18
> +        * Modules/modern-media-controls/controls/inline-media-controls.css: Invert the position of the two top controls

Most people use invert to mean flip vertically. I think "swap" makes more sense here, but I'm sure no one will be confused so don't worry.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:1
> +<?xml version="1.0" encoding="UTF-8"?>

No need for the encoding attribute.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:2
> +<svg width="22px" height="15px" viewBox="0 0 22 15" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

No need for the xlink namespace, or the version attribute. You should probably remove the width and height too.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:6
> +    <!-- Generator: Sketch 45 (43475) - http://www.bohemiancoding.com/sketch -->
> +    <title>_Assets/Both/Mute RTL</title>
> +    <desc>Created with Sketch.</desc>
> +    <defs></defs>

All this is unnecessary.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:7
> +    <g id="Media-Control-Symbols" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">

stroke="none" is redundant.

Also, no point setting stroke-width if there is no stroke. 

And why set fill to none and then the only child sets it back to black. Just remove that attribute.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:8
> +        <g id="_Assets/Both/Mute-RTL" fill="#000000">

No need for this if the parent g gets rid of its fill attribute.

Also, no need for the id attribute.

> Source/WebCore/Modules/modern-media-controls/images/iOS/Mute-RTL.svg:9
> +            <path d="M15.0384401,12.4336332 L14.7151198,12.7445465 C14.3273368,13.1174489 14,12.9886171 14,12.44141 L14,11.8340896 L15.0384401,12.4336332 Z M14,7.79263767 L14,2.56259782 C14,2.02224368 14.328475,1.87372699 14.7336617,2.24618642 L17.7297388,5.00026821 C17.7787854,5 17.8276579,5 17.877,5 L20.123,5 C20.195,5 20.266,5 20.338,5.00083333 C20.398,5.00083333 20.459,5.00166667 20.519,5.0025 C20.651,5.00583333 20.783,5.01166667 20.913,5.03166667 C21.045,5.05166667 21.168,5.08333333 21.288,5.13416667 C21.406,5.18416667 21.514,5.25 21.607,5.3275 C21.701,5.40583333 21.779,5.495 21.839,5.59333333 C21.9,5.69333333 21.939,5.79583333 21.962,5.90583333 C21.986,6.01416667 21.993,6.125 21.997,6.23416667 C21.999,6.28416667 21.999,6.335 21.999,6.385 C22,6.445 22,6.50416667 22,6.56416667 L22,8.43583333 C22,8.49583333 22,8.55583333 21.999,8.615 C21.999,8.66583333 21.999,8.71583333 21.997,8.76583333 C21.993,8.87583333 21.986,8.98583333 21.962,9.09416667 C21.939,9.20416667 21.9,9.30666667 21.839,9.40666667 C21.779,9.505 21.701,9.595 21.607,9.6725 C21.514,9.75083333 21.406,9.81583333 21.288,9.86583333 C21.168,9.91666667 21.045,9.94916667 20.913,9.96833333 C20.783,9.98833333 20.651,9.99416667 20.519,9.9975 C20.459,9.99916667 20.398,9.99916667 20.338,10 L20.123,10 L17.877,10 L17.8232637,10 L14,7.79263767 Z M12.0984016,10.7362012 C12.0719556,10.7963525 12.0306958,10.8516898 11.975,10.8969802 C11.786,11.0514833 11.502,11.0295606 11.34,10.848959 C11.0688063,10.5458841 10.8359472,10.228546 10.6440423,9.89652647 L12.0984016,10.7362012 Z M10.3551781,5.68829877 C10.5818609,5.1395229 10.9135304,4.62764579 11.34,4.15104101 C11.502,3.97043942 11.786,3.94851668 11.975,4.10301978 C12.165,4.25752287 12.187,4.52894723 12.026,4.70954882 C11.6287256,5.15341338 11.3293676,5.62791235 11.1397237,6.1412564 L10.3551781,5.68829877 Z M6.00498141,7.21816342 L6.90494133,7.73775552 C6.9605099,9.41225607 7.60202565,11.0012512 8.725,12.307171 C8.882,12.4897218 8.852,12.7568441 8.659,12.90536 C8.466,13.0528445 8.182,13.0249978 8.025,12.842447 C6.719,11.3242845 6,9.45649073 6,7.5 C6,7.40583933 6.00166538,7.31188411 6.00498141,7.21816342 Z M6.95104701,3.72292276 C7.24532664,3.17072984 7.60477673,2.64604244 8.025,2.15755298 C8.182,1.97500219 8.466,1.94715546 8.659,2.09464 C8.852,2.2431559 8.882,2.51027825 8.725,2.69282905 C8.32972705,3.15249651 7.99410451,3.64723565 7.72268909,4.16843052 L6.95104701,3.72292276 Z M2.26185384,5.05706771 L3.08281215,5.53104821 C2.96197404,6.16994634 2.901,6.82684524 2.901,7.5 C2.901,10.0310229 3.763,12.3322339 5.428,14.3108837 C5.583,14.4953516 5.551,14.7622632 5.355,14.9075703 C5.161,15.0539079 4.877,15.0229915 4.722,14.8395542 C2.929,12.7083835 2,10.2247657 2,7.5 C2,6.66178829 2.08791538,5.84639747 2.26185384,5.05706771 Z M3.57096454,1.77143124 C3.90321848,1.21476886 4.28720592,0.677244684 4.722,0.160445787 C4.877,-0.0229915406 5.161,-0.0539079442 5.355,0.0924296995 C5.551,0.237736796 5.583,0.504648414 5.428,0.689116289 C5.01602607,1.17869716 4.6532138,1.68802602 4.34045694,2.21569788 L3.57096454,1.77143124 Z M0.348205106,1.99951896 C0.555310106,1.64080257 1.00825831,1.51457545 1.37243073,1.7248305 L21.3782047,13.2751695 C21.7367591,13.4821809 21.8609811,13.9382988 21.6524304,14.2995191 C21.4453254,14.6582355 20.9923772,14.7844627 20.6282047,14.5742076 L0.622430735,3.0238686 C0.263876405,2.81685716 0.139654415,2.36073935 0.348205106,1.99951896 Z" id="iOS/Fullscreen/Mute" style="mix-blend-mode: normal;"></path>

This is way more significant digits than necessary.

Remove the mix-blend-mode style attribute. I don't even know why that is there.

And no need for the id attribute.

> Source/WebCore/Modules/modern-media-controls/images/iOS/VolumeHi-RTL.svg:1
> +<?xml version="1.0" encoding="UTF-8"?>

Ditto from the previous file.

> Source/WebCore/Modules/modern-media-controls/images/macOS/Mute-RTL.svg:1
> +<?xml version="1.0" encoding="UTF-8"?>

Ditto.

> Source/WebCore/Modules/modern-media-controls/images/macOS/VolumeHi-RTL.svg:1
> +<?xml version="1.0" encoding="UTF-8"?>

Ditto.

[Antoine Quint]

(In reply to Dean Jackson from comment #3)
> Comment on attachment 314161 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=314161&action=review
> 
> > Source/WebCore/ChangeLog:17
> > +        * Modules/modern-media-controls/controls/icon-service.js: Add new RTL variants for the mute and unmute icons.
> 
> Why didn't we just flip the existing ones?

Because while the speaker and waves are flipped, the crossing line isn't.

[WebKit Commit Bot]

Comment on attachment 314161 [details]
Patch

Clearing flags on attachment: 314161

Committed r218991: <http://trac.webkit.org/changeset/218991>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.