[Modern Media Controls] Don't use arrays as values for localisable strings
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Feb 2018 18:46:10 +0000 (18:46 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Feb 2018 18:46:10 +0000 (18:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182791
<rdar://problem/36007262>

Patch by Antoine Quint <graouts@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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228471 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/modern-media-controls/localized-strings/replaced-string-expected.txt
LayoutTests/media/modern-media-controls/localized-strings/replaced-string.html
Source/WebCore/ChangeLog
Source/WebCore/English.lproj/modern-media-controls-localized-strings.js
Source/WebCore/Modules/modern-media-controls/controls/icon-service.js
Source/WebCore/Modules/modern-media-controls/main.js
Source/WebCore/Modules/modern-media-controls/media/skip-back-support.js
Source/WebCore/Modules/modern-media-controls/media/skip-forward-support.js

index 5b46593..4f0c395 100644 (file)
@@ -1,3 +1,14 @@
+2018-02-14  Antoine Quint  <graouts@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  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r228444.
index 11b23be..7512c45 100644 (file)
@@ -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
index ef2e2e9..0a6745f 100644 (file)
@@ -3,12 +3,11 @@
 <script src="../../../../Source/WebCore/Modules/modern-media-controls/main.js" 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="../../../resources/js-test-post.js"></script>
index 4e2e0cb..77e35b3 100644 (file)
@@ -1,3 +1,24 @@
+2018-02-14  Antoine Quint  <graouts@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  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r228444.
index f7850fb..5254b2c 100644 (file)
@@ -1,5 +1,4 @@
 const UIStrings = {
-    "##SKIP_AMOUNT##": "15",
     "AirPlay": "AirPlay",
     "Audio": "Audio",
     "Audio Controls": "Audio Controls",
@@ -18,8 +17,8 @@ const UIStrings = {
     "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",
index 2a5052f..0c2937b 100644 (file)
@@ -35,8 +35,8 @@ const Icons = {
     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") },
index 0fdc263..823ac28 100644 (file)
@@ -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 @@ function createControls(shadowRoot, media, host)
     return new MediaController(shadowRoot, media, host);
 }
 
-function UIString(stringToLocalize)
+function UIString(stringToLocalize, replacementString)
 {
     let allLocalizedStrings = {};
     try {
@@ -48,9 +50,8 @@ function UIString(stringToLocalize)
     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;
 }
index 1101ed4..6c3bcef 100644 (file)
@@ -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 @@ class SkipBackSupport extends MediaControllerSupport
     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()
index 15b45d9..7f61c88 100644 (file)
@@ -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 @@ class SkipForwardSupport extends MediaControllerSupport
     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()