Title: [228471] trunk
Revision
228471
Author
commit-qu...@webkit.org
Date
2018-02-14 10:46:10 -0800 (Wed, 14 Feb 2018)

Log Message

[Modern Media Controls] Don't use arrays as values for localisable strings
https://bugs.webkit.org/show_bug.cgi?id=182791
<rdar://problem/36007262>

Patch by Antoine Quint <grao...@apple.com> on 2018-02-14
Reviewed by Dean Jackson.

Source/WebCore:

Localization tools expect localizable strings to be specified as key-value pairs where both the key and the pair
are plain strings. For the skip buttons, we used an array value to specify a replacement string. We now perform
this task in code with a centralized SkipSeconds constant defining the skip amount.

* English.lproj/modern-media-controls-localized-strings.js:
* Modules/modern-media-controls/controls/icon-service.js:
* Modules/modern-media-controls/main.js:
(UIString):
* Modules/modern-media-controls/media/skip-back-support.js:
(SkipBackSupport.prototype.buttonWasPressed):
* Modules/modern-media-controls/media/skip-forward-support.js:
(SkipForwardSupport.prototype.buttonWasPressed):

LayoutTests:

* media/modern-media-controls/localized-strings/replaced-string-expected.txt:
* media/modern-media-controls/localized-strings/replaced-string.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (228470 => 228471)


--- trunk/LayoutTests/ChangeLog	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/LayoutTests/ChangeLog	2018-02-14 18:46:10 UTC (rev 228471)
@@ -1,3 +1,14 @@
+2018-02-14  Antoine Quint  <grao...@apple.com>
+
+        [Modern Media Controls] Don't use arrays as values for localisable strings
+        https://bugs.webkit.org/show_bug.cgi?id=182791
+        <rdar://problem/36007262>
+
+        Reviewed by Dean Jackson.
+
+        * media/modern-media-controls/localized-strings/replaced-string-expected.txt:
+        * media/modern-media-controls/localized-strings/replaced-string.html:
+
 2018-02-14  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r228444.

Modified: trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string-expected.txt (228470 => 228471)


--- trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string-expected.txt	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string-expected.txt	2018-02-14 18:46:10 UTC (rev 228471)
@@ -1,9 +1,9 @@
-Testing that we can load a replaced string using UIString.
+Testing that we can replace strings using UIString.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS UIString('Test replaced string') is "Value 15 should be printed here"
+PASS UIString('Test replaced string', 15) is "Value 15 should be printed here"
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string.html (228470 => 228471)


--- trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string.html	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/LayoutTests/media/modern-media-controls/localized-strings/replaced-string.html	2018-02-14 18:46:10 UTC (rev 228471)
@@ -3,12 +3,11 @@
 <script src="" type="text/_javascript_"></script>
 <script type="text/_javascript_">
 
-description("Testing that we can load a replaced string using UIString.");
+description("Testing that we can replace strings using UIString.");
 
-UIStrings["##REPLACEMENT_VALUE##"] = "15";
-UIStrings["Test replaced string"] = ["Value %s should be printed here", "##REPLACEMENT_VALUE##"];
+UIStrings["Test replaced string"] = "Value %s should be printed here";
 
-shouldBeEqualToString("UIString('Test replaced string')", "Value 15 should be printed here");
+shouldBeEqualToString("UIString('Test replaced string', 15)", "Value 15 should be printed here");
 
 </script>
 <script src=""

Modified: trunk/Source/WebCore/ChangeLog (228470 => 228471)


--- trunk/Source/WebCore/ChangeLog	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/ChangeLog	2018-02-14 18:46:10 UTC (rev 228471)
@@ -1,3 +1,24 @@
+2018-02-14  Antoine Quint  <grao...@apple.com>
+
+        [Modern Media Controls] Don't use arrays as values for localisable strings
+        https://bugs.webkit.org/show_bug.cgi?id=182791
+        <rdar://problem/36007262>
+
+        Reviewed by Dean Jackson.
+
+        Localization tools expect localizable strings to be specified as key-value pairs where both the key and the pair
+        are plain strings. For the skip buttons, we used an array value to specify a replacement string. We now perform
+        this task in code with a centralized SkipSeconds constant defining the skip amount.
+
+        * English.lproj/modern-media-controls-localized-strings.js:
+        * Modules/modern-media-controls/controls/icon-service.js:
+        * Modules/modern-media-controls/main.js:
+        (UIString):
+        * Modules/modern-media-controls/media/skip-back-support.js:
+        (SkipBackSupport.prototype.buttonWasPressed):
+        * Modules/modern-media-controls/media/skip-forward-support.js:
+        (SkipForwardSupport.prototype.buttonWasPressed):
+
 2018-02-14  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r228444.

Modified: trunk/Source/WebCore/English.lproj/modern-media-controls-localized-strings.js (228470 => 228471)


--- trunk/Source/WebCore/English.lproj/modern-media-controls-localized-strings.js	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/English.lproj/modern-media-controls-localized-strings.js	2018-02-14 18:46:10 UTC (rev 228471)
@@ -1,5 +1,4 @@
 const UIStrings = {
-    "##SKIP_AMOUNT##": "15",
     "AirPlay": "AirPlay",
     "Audio": "Audio",
     "Audio Controls": "Audio Controls",
@@ -18,8 +17,8 @@
     "Play": "Play",
     "Remaining": "Remaining",
     "Rewind": "Rewind",
-    "Skip Back 15 seconds": ["Skip Back %s seconds", "##SKIP_AMOUNT##"],
-    "Skip Forward 15 seconds": ["Skip Forward %s seconds", "##SKIP_AMOUNT##"],
+    "Skip Back %s Seconds": "Skip Back %s Seconds",
+    "Skip Forward %s Seconds": "Skip Forward %s Seconds",
     "Subtitles": "Subtitles",
     "This video is playing in picture in picture.": "This video is playing in picture in picture.",
     "This video is playing on your Apple TV": "This video is playing on your Apple TV",

Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/icon-service.js (228470 => 228471)


--- trunk/Source/WebCore/Modules/modern-media-controls/controls/icon-service.js	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/icon-service.js	2018-02-14 18:46:10 UTC (rev 228471)
@@ -35,8 +35,8 @@
     PiPPlacard      : { name: "pip-placard", type: "png", label: UIString("Picture in Picture") },
     Play            : { name: "Play", type: "svg", label: UIString("Play") },
     Rewind          : { name: "Rewind", type: "svg", label: UIString("Rewind") },
-    SkipBack        : { name: "SkipBack15", type: "svg", label: UIString("Skip Back 15 seconds") },
-    SkipForward     : { name: "SkipForward15", type: "svg", label: UIString("Skip Forward 15 seconds") },
+    SkipBack        : { name: "SkipBack15", type: "svg", label: UIString("Skip Back %s Seconds", SkipSeconds) },
+    SkipForward     : { name: "SkipForward15", type: "svg", label: UIString("Skip Forward %s Seconds", SkipSeconds) },
     Tracks          : { name: "MediaSelector", type: "svg", label: UIString("Media Selection") },
     Volume          : { name: "VolumeHi", type: "svg", label: UIString("Mute") },
     VolumeRTL       : { name: "VolumeHi-RTL", type: "svg", label: UIString("Mute") },

Modified: trunk/Source/WebCore/Modules/modern-media-controls/main.js (228470 => 228471)


--- trunk/Source/WebCore/Modules/modern-media-controls/main.js	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/Modules/modern-media-controls/main.js	2018-02-14 18:46:10 UTC (rev 228471)
@@ -23,6 +23,8 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+const SkipSeconds = 15;
+
 let mediaControlsHost;
 
 // This is called from HTMLMediaElement::ensureMediaControlsInjectedScript().
@@ -37,7 +39,7 @@
     return new MediaController(shadowRoot, media, host);
 }
 
-function UIString(stringToLocalize)
+function UIString(stringToLocalize, replacementString)
 {
     let allLocalizedStrings = {};
     try {
@@ -48,9 +50,8 @@
     if (!localizedString)
         return stringToLocalize;
 
-    // We allow an array of a string and a replacement.
-    if (Array.isArray(localizedString) && localizedString.length == 2)
-        return localizedString[0].replace("%s", UIString(localizedString[1]));
+    if (replacementString)
+        return localizedString.replace("%s", replacementString);
 
     return localizedString;
 }

Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/skip-back-support.js (228470 => 228471)


--- trunk/Source/WebCore/Modules/modern-media-controls/media/skip-back-support.js	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/skip-back-support.js	2018-02-14 18:46:10 UTC (rev 228471)
@@ -23,8 +23,6 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-const SkipBackSeconds = 15;
-
 class SkipBackSupport extends MediaControllerSupport
 {
 
@@ -43,7 +41,7 @@
     buttonWasPressed(control)
     {
         const media = this.mediaController.media;
-        media.currentTime = Math.max(media.currentTime - SkipBackSeconds, media.seekable.start(0));
+        media.currentTime = Math.max(media.currentTime - SkipSeconds, media.seekable.start(0));
     }
 
     syncControl()

Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/skip-forward-support.js (228470 => 228471)


--- trunk/Source/WebCore/Modules/modern-media-controls/media/skip-forward-support.js	2018-02-14 18:34:42 UTC (rev 228470)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/skip-forward-support.js	2018-02-14 18:46:10 UTC (rev 228471)
@@ -23,8 +23,6 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-const SkipForwardSeconds = 15;
-
 class SkipForwardSupport extends MediaControllerSupport
 {
 
@@ -43,7 +41,7 @@
     buttonWasPressed(control)
     {
         const media = this.mediaController.media;
-        media.currentTime = Math.min(media.currentTime + SkipForwardSeconds, media.seekable.end(0));
+        media.currentTime = Math.min(media.currentTime + SkipSeconds, media.seekable.end(0));
     }
 
     syncControl()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to