commit 906ac577b228bc9be69e7f1b382756fbe0b4f6c6
author Ankit Kumar <ankk@microsoft.com> Tue Apr 21 05:58:04 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Apr 21 05:58:04 2020 +0000
tree aee39dcab6b7b7156523e2bcae98fc8e18f54479
parent 449f2fcd998db2dc579be6cceb9aaefb4bd7444e

Only set focus to WIDGET annotations if it's focusable

The widgets annotations are focusable by default. Since we have
introduced FPDFAnnot_SetFocusableSubtypes() API, we should check whether
WIDGET annotations are focusable or not.

Bug: pdfium:1507
Change-Id: I500a8a91ff76502bcc73d65a2e45dea12875054b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68611
Commit-Queue:  Ankit Kumar 🌪️ <ankk@microsoft.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>

fpdfsdk/cpdfsdk_widgethandler.cpp

diff --git a/fpdfsdk/cpdfsdk_widgethandler.cpp b/fpdfsdk/cpdfsdk_widgethandler.cpp
index 5881fd4..a56cce8 100644
--- a/fpdfsdk/cpdfsdk_widgethandler.cpp
+++ b/fpdfsdk/cpdfsdk_widgethandler.cpp
@@ -20,6 +20,7 @@
 #include "fpdfsdk/cpdfsdk_pageview.h"
 #include "fpdfsdk/cpdfsdk_widget.h"
 #include "fpdfsdk/formfiller/cffl_formfiller.h"
+#include "third_party/base/stl_util.h"
 
 CPDFSDK_WidgetHandler::CPDFSDK_WidgetHandler() = default;
 
@@ -222,12 +223,18 @@
 
 bool CPDFSDK_WidgetHandler::OnSetFocus(ObservedPtr<CPDFSDK_Annot>* pAnnot,
                                        uint32_t nFlag) {
+  if (!IsFocusableAnnot((*pAnnot)->GetPDFAnnot()->GetSubtype()))
+    return false;
+
   return (*pAnnot)->IsSignatureWidget() ||
          m_pFormFiller->OnSetFocus(pAnnot, nFlag);
 }
 
 bool CPDFSDK_WidgetHandler::OnKillFocus(ObservedPtr<CPDFSDK_Annot>* pAnnot,
                                         uint32_t nFlag) {
+  if (!IsFocusableAnnot((*pAnnot)->GetPDFAnnot()->GetSubtype()))
+    return false;
+
   return (*pAnnot)->IsSignatureWidget() ||
          m_pFormFiller->OnKillFocus(pAnnot, nFlag);
 }
@@ -293,3 +300,11 @@
   ASSERT(pAnnot);
   return GetViewBBox(pPageView, pAnnot).Contains(point);
 }
+
+bool CPDFSDK_WidgetHandler::IsFocusableAnnot(
+    const CPDF_Annot::Subtype& annot_type) const {
+  ASSERT(annot_type == CPDF_Annot::Subtype::WIDGET);
+
+  return pdfium::ContainsValue(m_pFormFillEnv->GetFocusableAnnotSubtypes(),
+                               annot_type);
+}

fpdfsdk/cpdfsdk_widgethandler.h

diff --git a/fpdfsdk/cpdfsdk_widgethandler.h b/fpdfsdk/cpdfsdk_widgethandler.h
index ebb6f6f..afbbc0d 100644
--- a/fpdfsdk/cpdfsdk_widgethandler.h
+++ b/fpdfsdk/cpdfsdk_widgethandler.h
@@ -101,6 +101,8 @@
   bool IsIndexSelected(ObservedPtr<CPDFSDK_Annot>* pAnnot, int index) override;
 
  private:
+  bool IsFocusableAnnot(const CPDF_Annot::Subtype& annot_type) const;
+
   UnownedPtr<CPDFSDK_FormFillEnvironment> m_pFormFillEnv;
   UnownedPtr<CFFL_InteractiveFormFiller> m_pFormFiller;
 };

fpdfsdk/fpdf_annot_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index 6dc02d2..0fb5084 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -2465,20 +2465,15 @@
       FPDFAnnot_SetFocusableSubtypes(form_handle(), kDefaultSubtypes, 0));
   ASSERT_EQ(0, FPDFAnnot_GetFocusableSubtypesCount(form_handle()));
 
-  // TODO(crbug.com/pdfium/1507): Widgets should not be focusable.
-  const FPDF_ANNOTATION_SUBTYPE kExpectedNoFocusableSubtypes[] = {
-      FPDF_ANNOT_WIDGET};
   VerifyAnnotationSubtypesAndFocusability(form_handle(), page,
-                                          kExpectedAnnotSubtypes,
-                                          kExpectedNoFocusableSubtypes);
+                                          kExpectedAnnotSubtypes, {});
 
   // Now make links focusable.
   const FPDF_ANNOTATION_SUBTYPE kLinkSubtypes[] = {FPDF_ANNOT_LINK};
   SetAndVerifyFocusableAnnotSubtypes(form_handle(), kLinkSubtypes);
 
-  // TODO(crbug.com/pdfium/1507): Widgets should not be focusable.
   const FPDF_ANNOTATION_SUBTYPE kExpectedLinkocusableSubtypes[] = {
-      FPDF_ANNOT_WIDGET, FPDF_ANNOT_LINK};
+      FPDF_ANNOT_LINK};
   VerifyAnnotationSubtypesAndFocusability(form_handle(), page,
                                           kExpectedAnnotSubtypes,
                                           kExpectedLinkocusableSubtypes);