Bug 1401821 - nsTextFormatter "*" width argument comes before the actual argument; r=froydnj
authorTom Tromey <tom@tromey.com>
Thu, 21 Sep 2017 09:43:28 -0600
changeset 382243 a5fec539b167cdedaf56e93b0a240fbb3c48f1c4
parent 382242 953afb698050792f6c3cde4e9fab84f8c6bf8450
child 382244 7e8e47c972f41335e9109c51597efdc3bca56910
push id32551
push userkwierso@gmail.com
push dateThu, 21 Sep 2017 23:29:53 +0000
treeherdermozilla-central@d6d6fd889f7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1401821, 1388789
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1401821 - nsTextFormatter "*" width argument comes before the actual argument; r=froydnj Bug 1388789 introduced a bug breaking formats like "%*.f". The problem was that the next "natural" argument was taken before the "*" width argument. MozReview-Commit-ID: BZack9faY7a
xpcom/string/nsTextFormatter.cpp
xpcom/tests/gtest/TestTextFormatter.cpp
--- a/xpcom/string/nsTextFormatter.cpp
+++ b/xpcom/string/nsTextFormatter.cpp
@@ -544,32 +544,16 @@ nsTextFormatter::dosprintf(SprintfStateS
         }
         sawNumberedArg = true;
       } else {
         width = argNumber;
         sawWidth = true;
       }
     }
 
-    if (thisArg == nullptr) {
-      // Mixing numbered arguments and implicit arguments is
-      // disallowed.
-      if (sawNumberedArg) {
-        return -1;
-      }
-
-      if (nextNaturalArg >= aValues.Length()) {
-        // A correctness issue but not a safety issue.
-        MOZ_ASSERT(false);
-        thisArg = &emptyString;
-      } else {
-        thisArg = &aValues[nextNaturalArg++];
-      }
-    }
-
     if (!sawWidth) {
       /*
        * Examine optional flags.  Note that we do not implement the
        * '#' flag of sprintf().  The ANSI C spec. of the '#' flag is
        * somewhat ambiguous and not ideal, which is perhaps why
        * the various sprintf() implementations are inconsistent
        * on this feature.
        */
@@ -640,16 +624,34 @@ nsTextFormatter::dosprintf(SprintfStateS
         prec = 0;
         while ((c >= '0') && (c <= '9')) {
           prec = (prec * 10) + (c - '0');
           c = *aFmt++;
         }
       }
     }
 
+    // If the argument isn't known yet, find it now.  This is done
+    // after the width and precision code, in case '*' was used.
+    if (thisArg == nullptr) {
+      // Mixing numbered arguments and implicit arguments is
+      // disallowed.
+      if (sawNumberedArg) {
+        return -1;
+      }
+
+      if (nextNaturalArg >= aValues.Length()) {
+        // A correctness issue but not a safety issue.
+        MOZ_ASSERT(false);
+        thisArg = &emptyString;
+      } else {
+        thisArg = &aValues[nextNaturalArg++];
+      }
+    }
+
     /* Size.  Defaults to 32 bits.  */
     uint64_t mask = UINT32_MAX;
     if (c == 'h') {
       c = *aFmt++;
       mask = UINT16_MAX;
     } else if (c == 'L') {
       c = *aFmt++;
       mask = UINT64_MAX;
--- a/xpcom/tests/gtest/TestTextFormatter.cpp
+++ b/xpcom/tests/gtest/TestTextFormatter.cpp
@@ -37,16 +37,20 @@ TEST(TextFormatter, Tests)
   EXPECT_STREQ("%1m!", NS_ConvertUTF16toUTF8(out2).get());
 
   // Treat NULL the same in both %s cases.
   nsTextFormatter::ssprintf(out2, u"%s %S", (char*) nullptr, (char16_t*) nullptr);
   EXPECT_STREQ("(null) (null)", NS_ConvertUTF16toUTF8(out2).get());
 
   nsTextFormatter::ssprintf(out2, u"%lld", INT64_MIN);
   EXPECT_STREQ("-9223372036854775808", NS_ConvertUTF16toUTF8(out2).get());
+
+  // Regression test for bug 1401821.
+  nsTextFormatter::ssprintf(out2, u"%*.f", 0, 23.2);
+  EXPECT_STREQ("23", NS_ConvertUTF16toUTF8(out2).get());
 }
 
 /*
  * Check misordered parameters
  */
 
 TEST(TextFormatterOrdering, orders)
 {