summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2016-01-29 08:55:30 -0800
committerTom Sepez <tsepez@chromium.org>2016-01-29 08:55:30 -0800
commit66519af52b61ca158044651d0507d47efb364f87 (patch)
treec0def01f0a7603ff7ae0dadd4bed6d3316f508f6
parentd45e7a51904164fb22049f0a7a80d2a94c06936b (diff)
downloadpdfium-66519af52b61ca158044651d0507d47efb364f87.tar.xz
Fix behaviour of app.alert() with a single object argument.
Centralize the "arguments in an object" logic. See the section "Method Arguments" in the js_api_reference.pdf. Add some tests to hit the ambiguous cases this section implies, and test that all parameters are passed end-to-end to our callbacks. R=thestig@chromium.org Review URL: https://codereview.chromium.org/1641693003 .
-rw-r--r--fpdfsdk/src/javascript/JS_Value.cpp33
-rw-r--r--fpdfsdk/src/javascript/JS_Value.h12
-rw-r--r--fpdfsdk/src/javascript/app.cpp110
-rw-r--r--samples/pdfium_test.cc13
-rw-r--r--testing/resources/javascript/app_alert.in26
-rw-r--r--testing/resources/javascript/app_alert_expected.txt14
6 files changed, 129 insertions, 79 deletions
diff --git a/fpdfsdk/src/javascript/JS_Value.cpp b/fpdfsdk/src/javascript/JS_Value.cpp
index cfa565e4d4..c63963a110 100644
--- a/fpdfsdk/src/javascript/JS_Value.cpp
+++ b/fpdfsdk/src/javascript/JS_Value.cpp
@@ -7,6 +7,7 @@
#include "JS_Value.h"
#include <time.h>
+#include <algorithm>
#include <cmath>
#include <limits>
@@ -879,3 +880,35 @@ bool JS_PortIsNan(double d) {
double JS_LocalTime(double d) {
return JS_GetDateTime() + _getDaylightSavingTA(d);
}
+
+std::vector<CJS_Value> JS_ExpandKeywordParams(
+ CJS_Runtime* pRuntime,
+ const std::vector<CJS_Value>& originals,
+ size_t nKeywords,
+ ...) {
+ ASSERT(nKeywords);
+
+ std::vector<CJS_Value> result(nKeywords, CJS_Value(pRuntime));
+ size_t size = std::min(originals.size(), nKeywords);
+ for (size_t i = 0; i < size; ++i)
+ result[i] = originals[i];
+
+ if (originals.size() != 1 || originals[0].GetType() != CJS_Value::VT_object ||
+ originals[0].IsArrayObject()) {
+ return result;
+ }
+ v8::Local<v8::Object> pObj = originals[0].ToV8Object();
+ result[0] = CJS_Value(pRuntime); // Make unknown.
+
+ va_list ap;
+ va_start(ap, nKeywords);
+ for (int i = 0; i < nKeywords; ++i) {
+ const wchar_t* property = va_arg(ap, const wchar_t*);
+ v8::Local<v8::Value> v8Value =
+ FXJS_GetObjectElement(pRuntime->GetIsolate(), pObj, property);
+ if (!v8Value->IsUndefined())
+ result[i] = CJS_Value(pRuntime, v8Value, CJS_Value::VT_unknown);
+ }
+ va_end(ap);
+ return result;
+}
diff --git a/fpdfsdk/src/javascript/JS_Value.h b/fpdfsdk/src/javascript/JS_Value.h
index 20a6e38b46..c33a973a12 100644
--- a/fpdfsdk/src/javascript/JS_Value.h
+++ b/fpdfsdk/src/javascript/JS_Value.h
@@ -213,4 +213,16 @@ double JS_MakeDate(double day, double time);
bool JS_PortIsNan(double d);
double JS_LocalTime(double d);
+// Some JS methods have the bizarre convention that they may also be called
+// with a single argument which is an object containing the actual arguments
+// as its properties. The varying arguments to this method are the property
+// names as wchar_t string literals corresponding to each positional argument.
+// The result will always contain |nKeywords| value, with unspecified ones
+// being set to type VT_unknown.
+std::vector<CJS_Value> JS_ExpandKeywordParams(
+ CJS_Runtime* pRuntime,
+ const std::vector<CJS_Value>& originals,
+ size_t nKeywords,
+ ...);
+
#endif // FPDFSDK_SRC_JAVASCRIPT_JS_VALUE_H_
diff --git a/fpdfsdk/src/javascript/app.cpp b/fpdfsdk/src/javascript/app.cpp
index 5959b9a1b5..d8be575918 100644
--- a/fpdfsdk/src/javascript/app.cpp
+++ b/fpdfsdk/src/javascript/app.cpp
@@ -246,90 +246,50 @@ FX_BOOL app::alert(IJS_Context* cc,
const std::vector<CJS_Value>& params,
CJS_Value& vRet,
CFX_WideString& sError) {
- int iSize = params.size();
- if (iSize < 1)
- return FALSE;
-
- CFX_WideString swMsg = L"";
- CFX_WideString swTitle = L"";
- int iIcon = 0;
- int iType = 0;
-
+ CJS_Context* pContext = static_cast<CJS_Context*>(cc);
CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc);
- v8::Isolate* isolate = pRuntime->GetIsolate();
+ std::vector<CJS_Value> newParams = JS_ExpandKeywordParams(
+ pRuntime, params, 4, L"cMsg", L"nIcon", L"nType", L"cTitle");
- if (iSize == 1) {
- if (params[0].GetType() == CJS_Value::VT_object) {
- v8::Local<v8::Object> pObj = params[0].ToV8Object();
- {
- v8::Local<v8::Value> pValue =
- FXJS_GetObjectElement(isolate, pObj, L"cMsg");
- swMsg = CJS_Value(pRuntime, pValue, CJS_Value::VT_unknown)
- .ToCFXWideString();
-
- pValue = FXJS_GetObjectElement(isolate, pObj, L"cTitle");
- swTitle = CJS_Value(pRuntime, pValue, CJS_Value::VT_unknown)
- .ToCFXWideString();
-
- pValue = FXJS_GetObjectElement(isolate, pObj, L"nIcon");
- iIcon = CJS_Value(pRuntime, pValue, CJS_Value::VT_unknown).ToInt();
-
- pValue = FXJS_GetObjectElement(isolate, pObj, L"nType");
- iType = CJS_Value(pRuntime, pValue, CJS_Value::VT_unknown).ToInt();
- }
-
- if (swMsg == L"") {
- CJS_Array carray(pRuntime);
- if (params[0].ConvertToArray(carray)) {
- int iLength = carray.GetLength();
- CJS_Value* pValue = new CJS_Value(pRuntime);
- for (int i = 0; i < iLength; ++i) {
- carray.GetElement(i, *pValue);
- swMsg += (*pValue).ToCFXWideString();
- if (i < iLength - 1)
- swMsg += L", ";
- }
+ if (newParams[0].GetType() == CJS_Value::VT_unknown) {
+ sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
+ return FALSE;
+ }
- delete pValue;
- }
+ CFX_WideString swMsg;
+ if (newParams[0].GetType() == CJS_Value::VT_object) {
+ CJS_Array carray(pRuntime);
+ if (newParams[0].ConvertToArray(carray)) {
+ swMsg = L"[";
+ CJS_Value element(pRuntime);
+ for (int i = 0; i < carray.GetLength(); ++i) {
+ if (i)
+ swMsg += L", ";
+ carray.GetElement(i, element);
+ swMsg += element.ToCFXWideString();
}
-
- if (swTitle == L"")
- swTitle = JSGetStringFromID((CJS_Context*)cc, IDS_STRING_JSALERT);
- } else if (params[0].GetType() == CJS_Value::VT_boolean) {
- FX_BOOL bGet = params[0].ToBool();
- if (bGet)
- swMsg = L"true";
- else
- swMsg = L"false";
-
- swTitle = JSGetStringFromID((CJS_Context*)cc, IDS_STRING_JSALERT);
+ swMsg += L"]";
} else {
- swMsg = params[0].ToCFXWideString();
- swTitle = JSGetStringFromID((CJS_Context*)cc, IDS_STRING_JSALERT);
+ swMsg = newParams[0].ToCFXWideString();
}
} else {
- if (params[0].GetType() == CJS_Value::VT_boolean) {
- FX_BOOL bGet = params[0].ToBool();
- if (bGet)
- swMsg = L"true";
- else
- swMsg = L"false";
- } else {
- swMsg = params[0].ToCFXWideString();
- }
- swTitle = JSGetStringFromID((CJS_Context*)cc, IDS_STRING_JSALERT);
-
- for (int i = 1; i < iSize; i++) {
- if (i == 1)
- iIcon = params[i].ToInt();
- if (i == 2)
- iType = params[i].ToInt();
- if (i == 3)
- swTitle = params[i].ToCFXWideString();
- }
+ swMsg = newParams[0].ToCFXWideString();
}
+ int iIcon = 0;
+ if (newParams[1].GetType() != CJS_Value::VT_unknown)
+ iIcon = newParams[1].ToInt();
+
+ int iType = 0;
+ if (newParams[2].GetType() != CJS_Value::VT_unknown)
+ iType = newParams[2].ToInt();
+
+ CFX_WideString swTitle;
+ if (newParams[3].GetType() != CJS_Value::VT_unknown)
+ swTitle = newParams[3].ToCFXWideString();
+ else
+ swTitle = JSGetStringFromID(pContext, IDS_STRING_JSALERT);
+
pRuntime->BeginBlock();
vRet = MsgBox(pRuntime->GetReaderApp(), swMsg.c_str(), swTitle.c_str(), iType,
iIcon);
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc
index 164364fbaf..3ad0e62626 100644
--- a/samples/pdfium_test.cc
+++ b/samples/pdfium_test.cc
@@ -195,10 +195,15 @@ void WriteEmf(FPDF_PAGE page, const char* pdf_name, int num) {
}
#endif
-int ExampleAppAlert(IPDF_JSPLATFORM*, FPDF_WIDESTRING msg, FPDF_WIDESTRING,
- int, int) {
- std::wstring platform_string = GetPlatformWString(msg);
- printf("Alert: %ls\n", platform_string.c_str());
+int ExampleAppAlert(IPDF_JSPLATFORM*,
+ FPDF_WIDESTRING msg,
+ FPDF_WIDESTRING title,
+ int nType,
+ int nIcon) {
+ printf("%ls", GetPlatformWString(title).c_str());
+ if (nIcon || nType)
+ printf("[icon=%d,type=%d]", nIcon, nType);
+ printf(": %ls\n", GetPlatformWString(msg).c_str());
return 0;
}
diff --git a/testing/resources/javascript/app_alert.in b/testing/resources/javascript/app_alert.in
index de6c8a8229..75aecc9075 100644
--- a/testing/resources/javascript/app_alert.in
+++ b/testing/resources/javascript/app_alert.in
@@ -35,6 +35,32 @@ endobj
>>
stream
app.alert("This test passes if alert() logs output under the test utility.");
+app.alert("message", 1, 2, "title");
+app.alert({"cMsg": "message", "cTitle": "title"});
+app.alert({"cMsg": "message", "cTitle": "title", "nIcon": 3, "nType": 4});
+app.alert(undefined);
+app.alert(null);
+app.alert(true);
+app.alert(false);
+app.alert(42);
+app.alert([1, 2, 3]);
+app.alert([1, 2, {"color": "red"}]);
+app.alert({"color": "red"}, 5, 6, "title");
+try {
+ app.alert();
+} catch (e) {
+ app.alert("Caught expected error " + e);
+}
+try {
+ app.alert({});
+} catch (e) {
+ app.alert("Caught expected error " + e);
+}
+try {
+ app.alert({"color": "red", "size": 42});
+} catch (e) {
+ app.alert("Caught expected error " + e);
+}
endstream
endobj
{{xref}}
diff --git a/testing/resources/javascript/app_alert_expected.txt b/testing/resources/javascript/app_alert_expected.txt
index 91205bc91d..b44fa73a94 100644
--- a/testing/resources/javascript/app_alert_expected.txt
+++ b/testing/resources/javascript/app_alert_expected.txt
@@ -1 +1,15 @@
Alert: This test passes if alert() logs output under the test utility.
+title[icon=1,type=2]: message
+title: message
+title[icon=3,type=4]: message
+Alert: undefined
+Alert: null
+Alert: true
+Alert: false
+Alert: 42
+Alert: [1, 2, 3]
+Alert: [1, 2, [object Object]]
+title[icon=5,type=6]: [object Object]
+Alert: Caught expected error app.alert: Incorrect number of parameters passed to function.
+Alert: Caught expected error app.alert: Incorrect number of parameters passed to function.
+Alert: Caught expected error app.alert: Incorrect number of parameters passed to function.