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,