From bd9325754999c2c3a01562ea090654f1ab07cc59 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 29 Jan 2016 09:10:41 -0800 Subject: Merge to XFA: Fix behaviour of app.alert() with a single object argument. Original Review URL: https://codereview.chromium.org/1641693003 . (cherry picked from commit 66519af52b61ca158044651d0507d47efb364f87) TBR=thestig@chromium.org Review URL: https://codereview.chromium.org/1639253008 . --- fpdfsdk/src/javascript/JS_Value.cpp | 33 +++++++ fpdfsdk/src/javascript/JS_Value.h | 12 +++ fpdfsdk/src/javascript/app.cpp | 110 +++++++-------------- samples/pdfium_test.cc | 13 ++- testing/resources/javascript/app_alert.in | 26 +++++ .../resources/javascript/app_alert_expected.txt | 14 +++ 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 +#include #include #include @@ -879,3 +880,35 @@ bool JS_PortIsNan(double d) { double JS_LocalTime(double d) { return JS_GetDateTime() + _getDaylightSavingTA(d); } + +std::vector JS_ExpandKeywordParams( + CJS_Runtime* pRuntime, + const std::vector& originals, + size_t nKeywords, + ...) { + ASSERT(nKeywords); + + std::vector 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 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 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 JS_ExpandKeywordParams( + CJS_Runtime* pRuntime, + const std::vector& 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 1473edbf69..692873e0f9 100644 --- a/fpdfsdk/src/javascript/app.cpp +++ b/fpdfsdk/src/javascript/app.cpp @@ -280,90 +280,50 @@ FX_BOOL app::alert(IJS_Context* cc, const std::vector& 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(cc); CJS_Runtime* pRuntime = CJS_Runtime::FromContext(cc); - v8::Isolate* isolate = pRuntime->GetIsolate(); + std::vector 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 pObj = params[0].ToV8Object(); - { - v8::Local 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 b1bce5172d..8c0a249f30 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. -- cgit v1.2.3