Allowing focus to move across pages
The CL modifies CPDFSDK_PageView::OnKeyDown() to handle the key down
events that come when there is no focused annotation.
The CL also modifies CPDFSDK_AnnotIterator::GetNextAnnot() and
CPDFSDK_AnnotIterator::GetPrevAnnot() to prevent tab cycle within a PDF
page. The CL also includes tests to validate tabbing behavior for a
PDF page with form fields.
Bug: chromium:989040
Change-Id: Iace6eef4704a66cf0db20946f7b8fd15b5f679f1
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/65810
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
index e5e0313..891e950 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -231,19 +231,35 @@
return GetAnnotHandler(pAnnot)->OnChar(pAnnot, nChar, nFlags);
}
-bool CPDFSDK_AnnotHandlerMgr::Annot_OnKeyDown(CPDFSDK_Annot* pAnnot,
+bool CPDFSDK_AnnotHandlerMgr::Annot_OnKeyDown(CPDFSDK_PageView* pPageView,
+ CPDFSDK_Annot* pAnnot,
int nKeyCode,
int nFlag) {
+ if (!pAnnot) {
+ // If pressed key is not tab then no action is needed.
+ if (nKeyCode != FWL_VKEY_Tab)
+ return false;
+
+ // If ctrl key or alt key is pressed, then no action is needed.
+ if (CPWL_Wnd::IsCTRLKeyDown(nFlag) || CPWL_Wnd::IsALTKeyDown(nFlag))
+ return false;
+
+ ObservedPtr<CPDFSDK_Annot> end_annot(GetFirstOrLastFocusableAnnot(
+ pPageView, CPWL_Wnd::IsSHIFTKeyDown(nFlag)));
+ return end_annot && pPageView->GetFormFillEnv()->SetFocusAnnot(&end_annot);
+ }
+
if (CPWL_Wnd::IsCTRLKeyDown(nFlag) || CPWL_Wnd::IsALTKeyDown(nFlag)) {
return GetAnnotHandler(pAnnot)->OnKeyDown(pAnnot, nKeyCode, nFlag);
}
ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pAnnot);
- CPDFSDK_PageView* pPageView = pAnnot->GetPageView();
CPDFSDK_Annot* pFocusAnnot = pPageView->GetFocusAnnot();
if (pFocusAnnot && (nKeyCode == FWL_VKEY_Tab)) {
ObservedPtr<CPDFSDK_Annot> pNext(
GetNextAnnot(pFocusAnnot, !CPWL_Wnd::IsSHIFTKeyDown(nFlag)));
- if (pNext && pNext.Get() != pFocusAnnot) {
+ if (!pNext)
+ return false;
+ if (pNext.Get() != pFocusAnnot) {
pPageView->GetFormFillEnv()->SetFocusAnnot(&pNext);
return true;
}
@@ -333,3 +349,20 @@
pFormFillEnv->GetFocusableAnnotSubtypes());
return bNext ? ai.GetNextAnnot(pSDKAnnot) : ai.GetPrevAnnot(pSDKAnnot);
}
+
+CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetFirstOrLastFocusableAnnot(
+ CPDFSDK_PageView* page_view,
+ bool last) const {
+#ifdef PDF_ENABLE_XFA
+ IPDF_Page* page = page_view->GetXFAPage();
+ if (page && !page->AsPDFPage()) {
+ // For XFA annots in XFA pages not backed by PDF pages.
+ return page->AsXFAPage()->GetFirstOrLastXFAAnnot(page_view, last);
+ }
+#endif // PDF_ENABLE_XFA
+
+ CPDFSDK_FormFillEnvironment* form_fill_env = page_view->GetFormFillEnv();
+ CPDFSDK_AnnotIterator ai(page_view,
+ form_fill_env->GetFocusableAnnotSubtypes());
+ return last ? ai.GetLastAnnot() : ai.GetFirstAnnot();
+}
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.h b/fpdfsdk/cpdfsdk_annothandlermgr.h
index cc3f715..a103469 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.h
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.h
@@ -97,7 +97,10 @@
uint32_t nFlags,
const CFX_PointF& point);
bool Annot_OnChar(CPDFSDK_Annot* pAnnot, uint32_t nChar, uint32_t nFlags);
- bool Annot_OnKeyDown(CPDFSDK_Annot* pAnnot, int nKeyCode, int nFlag);
+ bool Annot_OnKeyDown(CPDFSDK_PageView* pPageView,
+ CPDFSDK_Annot* pAnnot,
+ int nKeyCode,
+ int nFlag);
bool Annot_OnSetFocus(ObservedPtr<CPDFSDK_Annot>* pAnnot, uint32_t nFlag);
bool Annot_OnKillFocus(ObservedPtr<CPDFSDK_Annot>* pAnnot, uint32_t nFlag);
bool Annot_SetIndexSelected(ObservedPtr<CPDFSDK_Annot>* pAnnot,
@@ -123,6 +126,8 @@
IPDFSDK_AnnotHandler* GetAnnotHandlerOfType(
CPDF_Annot::Subtype nAnnotSubtype) const;
CPDFSDK_Annot* GetNextAnnot(CPDFSDK_Annot* pSDKAnnot, bool bNext);
+ CPDFSDK_Annot* GetFirstOrLastFocusableAnnot(CPDFSDK_PageView* page_view,
+ bool last) const;
// |m_pBAAnnotHandler| and |m_pWidgetHandler| are always present, but
// |m_pXFAWidgetHandler| is only present in XFA mode.
diff --git a/fpdfsdk/cpdfsdk_annotiterator.cpp b/fpdfsdk/cpdfsdk_annotiterator.cpp
index c1db6bb..1e0867a 100644
--- a/fpdfsdk/cpdfsdk_annotiterator.cpp
+++ b/fpdfsdk/cpdfsdk_annotiterator.cpp
@@ -70,16 +70,14 @@
return nullptr;
++iter;
if (iter == m_Annots.end())
- iter = m_Annots.begin();
+ return nullptr;
return *iter;
}
CPDFSDK_Annot* CPDFSDK_AnnotIterator::GetPrevAnnot(CPDFSDK_Annot* pAnnot) {
auto iter = std::find(m_Annots.begin(), m_Annots.end(), pAnnot);
- if (iter == m_Annots.end())
+ if (iter == m_Annots.begin() || iter == m_Annots.end())
return nullptr;
- if (iter == m_Annots.begin())
- iter = m_Annots.end();
return *(--iter);
}
diff --git a/fpdfsdk/cpdfsdk_annotiterator_embeddertest.cpp b/fpdfsdk/cpdfsdk_annotiterator_embeddertest.cpp
index 651e46b..556eb8b 100644
--- a/fpdfsdk/cpdfsdk_annotiterator_embeddertest.cpp
+++ b/fpdfsdk/cpdfsdk_annotiterator_embeddertest.cpp
@@ -55,7 +55,7 @@
pAnnot = iter.GetNextAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), LeftBottom);
pAnnot = iter.GetNextAnnot(pAnnot);
- EXPECT_EQ(iter.GetFirstAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
pAnnot = iter.GetLastAnnot();
CheckRect(pAnnot->GetRect(), LeftBottom);
@@ -66,7 +66,7 @@
pAnnot = iter.GetPrevAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), RightTop);
pAnnot = iter.GetPrevAnnot(pAnnot);
- EXPECT_EQ(iter.GetLastAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
}
{
// Page 1 specifies "column order"
@@ -81,7 +81,7 @@
pAnnot = iter.GetNextAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), LeftBottom);
pAnnot = iter.GetNextAnnot(pAnnot);
- EXPECT_EQ(iter.GetFirstAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
pAnnot = iter.GetLastAnnot();
CheckRect(pAnnot->GetRect(), LeftBottom);
@@ -92,7 +92,7 @@
pAnnot = iter.GetPrevAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), RightTop);
pAnnot = iter.GetPrevAnnot(pAnnot);
- EXPECT_EQ(iter.GetLastAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
}
{
// Page 2 specifies "struct order"
@@ -107,7 +107,7 @@
pAnnot = iter.GetNextAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), RightBottom);
pAnnot = iter.GetNextAnnot(pAnnot);
- EXPECT_EQ(iter.GetFirstAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
pAnnot = iter.GetLastAnnot();
CheckRect(pAnnot->GetRect(), RightBottom);
@@ -118,7 +118,7 @@
pAnnot = iter.GetPrevAnnot(pAnnot);
CheckRect(pAnnot->GetRect(), LeftBottom);
pAnnot = iter.GetPrevAnnot(pAnnot);
- EXPECT_EQ(iter.GetLastAnnot(), pAnnot);
+ EXPECT_EQ(nullptr, pAnnot);
}
UnloadPage(page2);
UnloadPage(page1);
diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp
index 45b3ffa..e63fca6 100644
--- a/fpdfsdk/cpdfsdk_pageview.cpp
+++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -467,12 +467,10 @@
}
bool CPDFSDK_PageView::OnKeyDown(int nKeyCode, int nFlag) {
- if (CPDFSDK_Annot* pAnnot = GetFocusAnnot()) {
- CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr =
- m_pFormFillEnv->GetAnnotHandlerMgr();
- return pAnnotHandlerMgr->Annot_OnKeyDown(pAnnot, nKeyCode, nFlag);
- }
- return false;
+ CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr =
+ m_pFormFillEnv->GetAnnotHandlerMgr();
+ return pAnnotHandlerMgr->Annot_OnKeyDown(this, GetFocusAnnot(), nKeyCode,
+ nFlag);
}
bool CPDFSDK_PageView::OnKeyUp(int nKeyCode, int nFlag) {
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index af0df63..d9c33fe 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -727,6 +727,181 @@
UnloadPage(page);
}
+TEST_F(FPDFFormFillEmbedderTest, FormFillFirstTab) {
+ ASSERT_TRUE(OpenDocument("annotiter.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+ int page_index = -2;
+ FPDF_ANNOTATION annot = nullptr;
+ EXPECT_TRUE(FORM_GetFocusedAnnot(form_handle(), &page_index, &annot));
+ EXPECT_EQ(0, page_index);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(1, FPDFPage_GetAnnotIndex(page, annot));
+ FPDFPage_CloseAnnot(annot);
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, FormFillFirstShiftTab) {
+ ASSERT_TRUE(OpenDocument("annotiter.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first shift-tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+ int page_index = -2;
+ FPDF_ANNOTATION annot = nullptr;
+ EXPECT_TRUE(FORM_GetFocusedAnnot(form_handle(), &page_index, &annot));
+ EXPECT_EQ(0, page_index);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(0, FPDFPage_GetAnnotIndex(page, annot));
+ FPDFPage_CloseAnnot(annot);
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, FormFillContinuousTab) {
+ ASSERT_TRUE(OpenDocument("annotiter.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ static constexpr int kExpectedAnnotIndex[] = {1, 2, 3, 0};
+ // Tabs should iterate focus over annotations.
+ for (size_t i = 0; i < FX_ArraySize(kExpectedAnnotIndex); ++i) {
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+ int page_index = -2;
+ FPDF_ANNOTATION annot = nullptr;
+ EXPECT_TRUE(FORM_GetFocusedAnnot(form_handle(), &page_index, &annot));
+ EXPECT_EQ(0, page_index);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(kExpectedAnnotIndex[i], FPDFPage_GetAnnotIndex(page, annot));
+ FPDFPage_CloseAnnot(annot);
+ }
+
+ // Tab should not be handled as the last annotation of the page is in focus.
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, FormFillContinuousShiftTab) {
+ ASSERT_TRUE(OpenDocument("annotiter.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ static constexpr int kExpectedAnnotIndex[] = {0, 3, 2, 1};
+ // Shift-tabs should iterate focus over annotations.
+ for (size_t i = 0; i < FX_ArraySize(kExpectedAnnotIndex); ++i) {
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+ int page_index = -2;
+ FPDF_ANNOTATION annot = nullptr;
+ EXPECT_TRUE(FORM_GetFocusedAnnot(form_handle(), &page_index, &annot));
+ EXPECT_EQ(0, page_index);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(kExpectedAnnotIndex[i], FPDFPage_GetAnnotIndex(page, annot));
+ FPDFPage_CloseAnnot(annot);
+ }
+
+ // Shift-tab should not be handled as the first annotation of the page is in
+ // focus.
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, TabWithModifiers) {
+ EXPECT_TRUE(OpenDocument("annotiter.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ControlKey));
+
+ ASSERT_FALSE(
+ FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, FWL_EVENTFLAG_AltKey));
+
+ ASSERT_FALSE(
+ FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ (FWL_EVENTFLAG_ControlKey | FWL_EVENTFLAG_ShiftKey)));
+
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ (FWL_EVENTFLAG_AltKey | FWL_EVENTFLAG_ShiftKey)));
+
+ UnloadPage(page);
+}
+
+#ifdef PDF_ENABLE_XFA
+TEST_F(FPDFFormFillEmbedderTest, XFAFormFillFirstTab) {
+ EXPECT_TRUE(OpenDocument("xfa/email_recommended.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, XFAFormFillFirstShiftTab) {
+ EXPECT_TRUE(OpenDocument("xfa/email_recommended.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first shift-tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, XFAFormFillContinuousTab) {
+ EXPECT_TRUE(OpenDocument("xfa/email_recommended.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+
+ // Subsequent tabs should move focus over annotations.
+ for (size_t i = 0; i < 9; ++i)
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+
+ // Tab should not be handled as the last annotation of the page is in focus.
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab, 0));
+
+ UnloadPage(page);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, XFAFormFillContinuousShiftTab) {
+ EXPECT_TRUE(OpenDocument("xfa/email_recommended.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ // Invoking first shift-tab on the page.
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+
+ // Subsequent shift-tabs should move focus over annotations.
+ for (size_t i = 0; i < 9; ++i) {
+ ASSERT_TRUE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+ }
+
+ // Shift-tab should not be handled as the first annotation of the page is in
+ // focus.
+ ASSERT_FALSE(FORM_OnKeyDown(form_handle(), page, FWL_VKEY_Tab,
+ FWL_EVENTFLAG_ShiftKey));
+
+ UnloadPage(page);
+}
+#endif // PDF_ENABLE_XFA
+
class DoURIActionBlockedDelegate final : public EmbedderTest::Delegate {
public:
void DoURIAction(FPDF_BYTESTRING uri) override {
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
index 9797429..d9868ab 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
@@ -196,12 +196,26 @@
CXFA_FFWidget* hNextFocus =
bNext ? pWidgetIterator->MoveToNext() : pWidgetIterator->MoveToPrevious();
- if (!hNextFocus && pSDKAnnot)
- hNextFocus = pWidgetIterator->MoveToFirst();
+ if (!hNextFocus)
+ return nullptr;
return pPageView->GetAnnotByXFAWidget(hNextFocus);
}
+CPDFSDK_Annot* CPDFXFA_Page::GetFirstOrLastXFAAnnot(CPDFSDK_PageView* page_view,
+ bool last) const {
+ CXFA_FFPageView* xfa_page_view = GetXFAPageView();
+ if (!xfa_page_view)
+ return nullptr;
+ std::unique_ptr<IXFA_WidgetIterator> it =
+ xfa_page_view->CreateTraverseWidgetIterator(XFA_WidgetStatus_Visible |
+ XFA_WidgetStatus_Viewable |
+ XFA_WidgetStatus_Focused);
+
+ return page_view->GetAnnotByXFAWidget(last ? it->MoveToLast()
+ : it->MoveToFirst());
+}
+
int CPDFXFA_Page::HasFormFieldAtPoint(const CFX_PointF& point) const {
CXFA_FFPageView* pPageView = GetXFAPageView();
if (!pPageView)
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.h b/fpdfsdk/fpdfxfa/cpdfxfa_page.h
index 73d5421..c121bab 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_page.h
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.h
@@ -19,6 +19,7 @@
class CPDF_Dictionary;
class CPDF_Document;
class CPDFSDK_Annot;
+class CPDFSDK_PageView;
class CXFA_FFPageView;
class CPDFXFA_Page final : public IPDF_Page {
@@ -48,6 +49,8 @@
void SetXFAPageViewIndex(int index) { m_iPageIndex = index; }
CXFA_FFPageView* GetXFAPageView() const;
CPDFSDK_Annot* GetNextXFAAnnot(CPDFSDK_Annot* pSDKAnnot, bool bNext);
+ CPDFSDK_Annot* GetFirstOrLastXFAAnnot(CPDFSDK_PageView* page_view,
+ bool last) const;
int HasFormFieldAtPoint(const CFX_PointF& point) const;
void DrawFocusAnnot(CFX_RenderDevice* pDevice,
CPDFSDK_Annot* pAnnot,
