commit 2e8701bf5c0703e8d76ad77223a99dd8b8fe6b6f
author Dan Sinclair <dsinclair@chromium.org> Wed Apr 04 02:24:20 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Apr 04 02:24:20 2018 +0000
tree 7c108e9a6f7934a6f3f8deefbd7fb0dae296863f
parent e99a1901e09ba467ffa5657f176bebe649e25b9d

[Merge M66] Handle inserting negative values without a selection

When handling the AFNumber_Keystroke callback, if the changed value is
negative and there is no selection, we'd incorrectly error out thinking
we were inserting before the negative sign.

This CL checks if the selection string is empty before erroring out.

TBR=hnakashima@chromium.org

Bug: chromium:817433
Change-Id: I0ca08e121e9a8c207e9c7793f5b8a4e795c135e6
Reviewed-on: https://pdfium-review.googlesource.com/28110
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
(cherry picked from commit c2df008640f56631735dd3ae89e118c9c56cd36d)
Reviewed-on: https://pdfium-review.googlesource.com/29730
Reviewed-by: dsinclair <dsinclair@chromium.org>

fxjs/cjs_publicmethods.cpp

diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp
index 74428c3..93015bd 100644
--- a/fxjs/cjs_publicmethods.cpp
+++ b/fxjs/cjs_publicmethods.cpp
@@ -1009,8 +1009,8 @@
 
   bool bHasSign = wstrValue.Contains(L'-') && !wstrSelected.Contains(L'-');
   if (bHasSign) {
-    // can't insert "change" in front to sign postion.
-    if (pEvent->SelStart() == 0) {
+    // can't insert "change" in front of sign position.
+    if (!wstrSelected.IsEmpty() && pEvent->SelStart() == 0) {
       pEvent->Rc() = false;
       return CJS_Return(true);
     }

fxjs/cjs_publicmethods_embeddertest.cpp

diff --git a/fxjs/cjs_publicmethods_embeddertest.cpp b/fxjs/cjs_publicmethods_embeddertest.cpp
index 874f59f..90efee9 100644
--- a/fxjs/cjs_publicmethods_embeddertest.cpp
+++ b/fxjs/cjs_publicmethods_embeddertest.cpp
@@ -196,3 +196,47 @@
   ASSERT_TRUE(!ret.HasReturn());
   ASSERT_EQ(L"7", result);
 }
+
+TEST_F(CJS_PublicMethodsEmbedderTest, AFNumber_Keystroke) {
+  v8::Isolate::Scope isolate_scope(isolate());
+  v8::HandleScope handle_scope(isolate());
+  v8::Context::Scope context_scope(GetV8Context());
+
+  EXPECT_TRUE(OpenDocument("calculate.pdf"));
+  auto* page = LoadPage(0);
+  ASSERT_TRUE(page);
+
+  CJS_Runtime runtime(static_cast<CPDFSDK_FormFillEnvironment*>(form_handle()));
+  runtime.NewEventContext();
+
+  auto* handler = runtime.GetCurrentEventContext()->GetEventHandler();
+
+  bool valid = true;
+  WideString result = L"-10";
+  WideString change = L"";
+
+  handler->m_pValue = &result;
+  handler->m_pbRc = &valid;
+  handler->m_pWideStrChange = &change;
+
+  handler->m_bWillCommit = 0;
+  handler->SetSelStart(0);
+  handler->SetSelEnd(0);
+
+  std::vector<v8::Local<v8::Value>> params;
+  params.push_back(runtime.NewString(L"-10"));
+  params.push_back(runtime.NewString(L""));
+
+  CJS_Return ret = CJS_PublicMethods::AFNumber_Keystroke(&runtime, params);
+  EXPECT_TRUE(valid);
+  EXPECT_TRUE(!ret.HasError());
+  EXPECT_TRUE(!ret.HasReturn());
+
+  UnloadPage(page);
+
+  // Keep the *SAN bots happy. One of these is an UnownedPtr, another seems to
+  // used during destruction. Clear them all to be safe and consistent.
+  handler->m_pValue = nullptr;
+  handler->m_pbRc = nullptr;
+  handler->m_pWideStrChange = nullptr;
+}