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>

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,