summaryrefslogtreecommitdiff
path: root/xfa/fxfa
diff options
context:
space:
mode:
authorRyan Harrison <rharrison@chromium.org>2018-07-05 16:20:28 +0000
committerChromium commit bot <commit-bot@chromium.org>2018-07-05 16:20:28 +0000
commit3fab31fb8e35eca693322ac292228e993b508102 (patch)
tree1a3e5986b50f9fad72a17258199c76f5862841a1 /xfa/fxfa
parent86b4f67d40c351ea8e67ba7b7dcc9d8dd7ad371e (diff)
downloadpdfium-3fab31fb8e35eca693322ac292228e993b508102.tar.xz
Clean up ProcessFormatTestValidate
The existing implementation was overly complex and hard to understand, so this simplifies the logic. As part of this a number of issues, such as the lack of a failure path, have been resolved. The spec implies that a picture clause of "" should only accept the empty string, but existing implementation returns that validation does exist. This is due to the GetPicture() method returning a string, instead of a pointer or an Optional, so there is no mechanism to differentiate between a clause with an empty string and the clause not being present. This CL maintains the existing behaviour, because there may be code elsewhere that depends on it. The existing implementation returns validation not existing if the node under test is non-interactive. Though this seems intuitively correct, it is problematic, because this logic isn't just called via an interaction with the node. The validity check could be initiated by a call from JS attached to an event or a different node. Thus it is possible for a node under test to be non-interactive, but the result of the test is still important. The caller may make the node interactive if it isn't valid, for example. The spec doesn't state anything about interactivity controlling if validity checks run. This CL removes the related logic, since it was causing issues with an example PDF from the wild. BUG=pdfium:1065 Change-Id: Icb5b97e0d90f6fbc7ad6b87d81e256803c757eb0 Reviewed-on: https://pdfium-review.googlesource.com/37131 Commit-Queue: Ryan Harrison <rharrison@chromium.org> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Diffstat (limited to 'xfa/fxfa')
-rw-r--r--xfa/fxfa/parser/cxfa_node.cpp89
1 files changed, 45 insertions, 44 deletions
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 86644694ca..10a022a82e 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2070,55 +2070,56 @@ void CXFA_Node::ProcessScriptTestValidate(CXFA_FFDocView* docView,
int32_t CXFA_Node::ProcessFormatTestValidate(CXFA_FFDocView* docView,
CXFA_Validate* validate,
bool bVersionFlag) {
+ WideString wsPicture = validate->GetPicture();
+ if (wsPicture.IsEmpty())
+ return XFA_EVENTERROR_NotExist;
+
WideString wsRawValue = GetRawValue();
- if (!wsRawValue.IsEmpty()) {
- WideString wsPicture = validate->GetPicture();
- if (wsPicture.IsEmpty())
- return XFA_EVENTERROR_NotExist;
+ if (wsRawValue.IsEmpty())
+ return XFA_EVENTERROR_Error;
- LocaleIface* pLocale = GetLocale();
- if (!pLocale)
- return XFA_EVENTERROR_NotExist;
-
- CXFA_LocaleValue lcValue = XFA_GetLocaleValue(this);
- if (!lcValue.ValidateValue(lcValue.GetValue(), wsPicture, pLocale,
- nullptr)) {
- IXFA_AppProvider* pAppProvider =
- docView->GetDoc()->GetApp()->GetAppProvider();
- if (!pAppProvider)
- return XFA_EVENTERROR_NotExist;
+ LocaleIface* pLocale = GetLocale();
+ if (!pLocale)
+ return XFA_EVENTERROR_NotExist;
- WideString wsFormatMsg = validate->GetFormatMessageText();
- WideString wsTitle = pAppProvider->GetAppTitle();
- if (validate->GetFormatTest() == XFA_AttributeEnum::Error) {
- if (wsFormatMsg.IsEmpty())
- wsFormatMsg = GetValidateMessage(true, bVersionFlag);
- pAppProvider->MsgBox(wsFormatMsg, wsTitle,
- static_cast<uint32_t>(AlertIcon::kError),
- static_cast<uint32_t>(AlertButton::kOK));
- return XFA_EVENTERROR_Success;
- }
- if (IsUserInteractive())
- return XFA_EVENTERROR_NotExist;
- if (wsFormatMsg.IsEmpty())
- wsFormatMsg = GetValidateMessage(false, bVersionFlag);
+ CXFA_LocaleValue lcValue = XFA_GetLocaleValue(this);
+ if (lcValue.ValidateValue(lcValue.GetValue(), wsPicture, pLocale, nullptr))
+ return XFA_EVENTERROR_Success;
- if (bVersionFlag) {
- pAppProvider->MsgBox(wsFormatMsg, wsTitle,
- static_cast<uint32_t>(AlertIcon::kWarning),
- static_cast<uint32_t>(AlertButton::kOK));
- return XFA_EVENTERROR_Success;
- }
- if (pAppProvider->MsgBox(wsFormatMsg, wsTitle,
- static_cast<uint32_t>(AlertIcon::kWarning),
- static_cast<uint32_t>(AlertButton::kYesNo)) ==
- static_cast<uint32_t>(AlertReturn::kYes)) {
- SetFlag(XFA_NodeFlag_UserInteractive);
- }
- return XFA_EVENTERROR_Success;
- }
+ IXFA_AppProvider* pAppProvider =
+ docView->GetDoc()->GetApp()->GetAppProvider();
+ if (!pAppProvider)
+ return XFA_EVENTERROR_NotExist;
+
+ WideString wsFormatMsg = validate->GetFormatMessageText();
+ WideString wsTitle = pAppProvider->GetAppTitle();
+ if (validate->GetFormatTest() == XFA_AttributeEnum::Error) {
+ if (wsFormatMsg.IsEmpty())
+ wsFormatMsg = GetValidateMessage(true, bVersionFlag);
+ pAppProvider->MsgBox(wsFormatMsg, wsTitle,
+ static_cast<uint32_t>(AlertIcon::kError),
+ static_cast<uint32_t>(AlertButton::kOK));
+ return XFA_EVENTERROR_Error;
}
- return XFA_EVENTERROR_NotExist;
+
+ if (wsFormatMsg.IsEmpty())
+ wsFormatMsg = GetValidateMessage(false, bVersionFlag);
+
+ if (bVersionFlag) {
+ pAppProvider->MsgBox(wsFormatMsg, wsTitle,
+ static_cast<uint32_t>(AlertIcon::kWarning),
+ static_cast<uint32_t>(AlertButton::kOK));
+ return XFA_EVENTERROR_Error;
+ }
+
+ if (pAppProvider->MsgBox(wsFormatMsg, wsTitle,
+ static_cast<uint32_t>(AlertIcon::kWarning),
+ static_cast<uint32_t>(AlertButton::kYesNo)) ==
+ static_cast<uint32_t>(AlertReturn::kYes)) {
+ SetFlag(XFA_NodeFlag_UserInteractive);
+ }
+
+ return XFA_EVENTERROR_Error;
}
int32_t CXFA_Node::ProcessNullTestValidate(CXFA_FFDocView* docView,