Remove bool arg from CPDFSDK_AnnotHandlerMgr::GetNextAnnot(). Introduce separate methods instead of using a boolean arg to control the behaviour of the method. Do the same for GetFirstOrLastFocusableAnnot(), and introduce some helper functions to minimize the inevitable bloat. Change-Id: Iabe94474bc34566d4c3787682f27289d8c588e0d Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/80936 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp index 980bf68..757aed3 100644 --- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp +++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -27,6 +27,15 @@ #include "fpdfsdk/fpdfxfa/cpdfxfa_page.h" #include "fpdfsdk/fpdfxfa/cpdfxfa_widget.h" #include "fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h" + +namespace { + +CPDFXFA_Page* XFAPageIfNotBackedByPDFPage(CPDFSDK_PageView* pPageView) { + auto* pPage = static_cast<CPDFXFA_Page*>(pPageView->GetXFAPage()); + return pPage && !pPage->AsPDFPage() ? pPage : nullptr; +} + +} // namespace #endif // PDF_ENABLE_XFA CPDFSDK_AnnotHandlerMgr::CPDFSDK_AnnotHandlerMgr( @@ -248,8 +257,9 @@ if (CPWL_Wnd::IsCTRLKeyDown(nFlag) || CPWL_Wnd::IsALTKeyDown(nFlag)) return false; - ObservedPtr<CPDFSDK_Annot> end_annot(GetFirstOrLastFocusableAnnot( - pPageView, CPWL_Wnd::IsSHIFTKeyDown(nFlag))); + ObservedPtr<CPDFSDK_Annot> end_annot( + CPWL_Wnd::IsSHIFTKeyDown(nFlag) ? GetLastFocusableAnnot(pPageView) + : GetFirstFocusableAnnot(pPageView)); return end_annot && pPageView->GetFormFillEnv()->SetFocusAnnot(&end_annot); } @@ -259,8 +269,9 @@ ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pAnnot); CPDFSDK_Annot* pFocusAnnot = pPageView->GetFocusAnnot(); if (pFocusAnnot && (nKeyCode == FWL_VKEY_Tab)) { - ObservedPtr<CPDFSDK_Annot> pNext( - GetNextAnnot(pFocusAnnot, !CPWL_Wnd::IsSHIFTKeyDown(nFlag))); + ObservedPtr<CPDFSDK_Annot> pNext(CPWL_Wnd::IsSHIFTKeyDown(nFlag) + ? GetPrevAnnot(pFocusAnnot) + : GetNextAnnot(pFocusAnnot)); if (!pNext) return false; if (pNext.Get() != pFocusAnnot) { @@ -336,37 +347,52 @@ return false; } -CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetNextAnnot(CPDFSDK_Annot* pSDKAnnot, - bool bNext) { +CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetNextAnnot( + CPDFSDK_Annot* pSDKAnnot) const { + CPDFSDK_PageView* pPageView = pSDKAnnot->GetPageView(); #ifdef PDF_ENABLE_XFA - IPDF_Page* pPage = pSDKAnnot->GetPageView()->GetXFAPage(); - if (pPage && !pPage->AsPDFPage()) { - // For xfa annots in XFA pages not backed by PDF pages. - return static_cast<CPDFXFA_Page*>(pPage)->GetNextXFAAnnot(pSDKAnnot, bNext); - } + CPDFXFA_Page* pXFAPage = XFAPageIfNotBackedByPDFPage(pPageView); + if (pXFAPage) + return pXFAPage->GetNextXFAAnnot(pSDKAnnot); #endif // PDF_ENABLE_XFA - CPDFSDK_FormFillEnvironment* pFormFillEnv = - pSDKAnnot->GetPageView()->GetFormFillEnv(); - - // For PDF annots. - CPDFSDK_AnnotIterator ai(pSDKAnnot->GetPageView(), - pFormFillEnv->GetFocusableAnnotSubtypes()); - return bNext ? ai.GetNextAnnot(pSDKAnnot) : ai.GetPrevAnnot(pSDKAnnot); + CPDFSDK_AnnotIterator ai( + pPageView, pPageView->GetFormFillEnv()->GetFocusableAnnotSubtypes()); + return ai.GetNextAnnot(pSDKAnnot); } -CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetFirstOrLastFocusableAnnot( - CPDFSDK_PageView* page_view, - bool last) const { +CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetPrevAnnot( + CPDFSDK_Annot* pSDKAnnot) const { + CPDFSDK_PageView* pPageView = pSDKAnnot->GetPageView(); #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); - } + CPDFXFA_Page* pXFAPage = XFAPageIfNotBackedByPDFPage(pPageView); + if (pXFAPage) + return pXFAPage->GetPrevXFAAnnot(pSDKAnnot); #endif // PDF_ENABLE_XFA + CPDFSDK_AnnotIterator ai( + pPageView, pPageView->GetFormFillEnv()->GetFocusableAnnotSubtypes()); + return ai.GetPrevAnnot(pSDKAnnot); +} - CPDFSDK_FormFillEnvironment* form_fill_env = page_view->GetFormFillEnv(); - CPDFSDK_AnnotIterator ai(page_view, - form_fill_env->GetFocusableAnnotSubtypes()); - return last ? ai.GetLastAnnot() : ai.GetFirstAnnot(); +CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetFirstFocusableAnnot( + CPDFSDK_PageView* page_view) const { +#ifdef PDF_ENABLE_XFA + CPDFXFA_Page* pXFAPage = XFAPageIfNotBackedByPDFPage(page_view); + if (pXFAPage) + return pXFAPage->GetFirstXFAAnnot(page_view); +#endif // PDF_ENABLE_XFA + CPDFSDK_AnnotIterator ai( + page_view, page_view->GetFormFillEnv()->GetFocusableAnnotSubtypes()); + return ai.GetFirstAnnot(); +} + +CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetLastFocusableAnnot( + CPDFSDK_PageView* page_view) const { +#ifdef PDF_ENABLE_XFA + CPDFXFA_Page* pXFAPage = XFAPageIfNotBackedByPDFPage(page_view); + if (pXFAPage) + return pXFAPage->GetLastXFAAnnot(page_view); +#endif // PDF_ENABLE_XFA + CPDFSDK_AnnotIterator ai( + page_view, page_view->GetFormFillEnv()->GetFocusableAnnotSubtypes()); + return ai.GetLastAnnot(); }
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.h b/fpdfsdk/cpdfsdk_annothandlermgr.h index c7dea78..a7f8b97 100644 --- a/fpdfsdk/cpdfsdk_annothandlermgr.h +++ b/fpdfsdk/cpdfsdk_annothandlermgr.h
@@ -126,9 +126,10 @@ IPDFSDK_AnnotHandler* GetAnnotHandler(CPDFSDK_Annot* pAnnot) const; 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; + CPDFSDK_Annot* GetNextAnnot(CPDFSDK_Annot* pSDKAnnot) const; + CPDFSDK_Annot* GetPrevAnnot(CPDFSDK_Annot* pSDKAnnot) const; + CPDFSDK_Annot* GetFirstFocusableAnnot(CPDFSDK_PageView* page_view) const; + CPDFSDK_Annot* GetLastFocusableAnnot(CPDFSDK_PageView* page_view) const; // |m_pBAAnnotHandler| and |m_pWidgetHandler| are always present, but // |m_pXFAWidgetHandler| is only present in XFA mode.
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp index bcbdd85..a707471 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
@@ -28,6 +28,42 @@ XFA_WidgetStatus_Viewable | XFA_WidgetStatus_Focused; +IXFA_WidgetIterator* GCedWidgetIteratorForPage(CXFA_FFPageView* pFFPageView, + CPDFSDK_PageView* pPageView) { + if (!pFFPageView) + return nullptr; + + ObservedPtr<CPDFSDK_PageView> pWatchedPageView(pPageView); + IXFA_WidgetIterator* pIterator = + pFFPageView->CreateGCedTraverseWidgetIterator(kIteratorFilter); + + // Check |pPageView| again because JS may have destroyed it. + return pWatchedPageView ? pIterator : nullptr; +} + +IXFA_WidgetIterator* GCedWidgetIteratorForAnnot(CXFA_FFPageView* pFFPageView, + CPDFSDK_Annot* pSDKAnnot) { + if (!pFFPageView) + return nullptr; + + CPDFXFA_Widget* pXFAWidget = ToXFAWidget(pSDKAnnot); + if (!pXFAWidget) + return nullptr; + + ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pSDKAnnot); + IXFA_WidgetIterator* pWidgetIterator = + pFFPageView->CreateGCedTraverseWidgetIterator(kIteratorFilter); + + // Check |pSDKAnnot| again because JS may have destroyed it. + if (!pObservedAnnot) + return nullptr; + + if (pWidgetIterator->GetCurrentWidget() != pXFAWidget->GetXFAFFWidget()) + pWidgetIterator->SetCurrentWidget(pXFAWidget->GetXFAFFWidget()); + + return pWidgetIterator; +} + } // namespace CPDFXFA_Page::CPDFXFA_Page(CPDF_Document* pDocument, int page_index) @@ -182,50 +218,44 @@ return CFX_Matrix(); } -CPDFSDK_Annot* CPDFXFA_Page::GetNextXFAAnnot(CPDFSDK_Annot* pSDKAnnot, - bool bNext) { - CPDFXFA_Widget* pXFAWidget = ToXFAWidget(pSDKAnnot); - if (!pXFAWidget) - return nullptr; - - CXFA_FFPageView* xfa_page_view = GetXFAPageView(); - if (!xfa_page_view) - return nullptr; - - ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pSDKAnnot); - CPDFSDK_PageView* pPageView = pSDKAnnot->GetPageView(); +CPDFSDK_Annot* CPDFXFA_Page::GetNextXFAAnnot(CPDFSDK_Annot* pSDKAnnot) const { IXFA_WidgetIterator* pWidgetIterator = - xfa_page_view->CreateGCedTraverseWidgetIterator(kIteratorFilter); - - // Check |pSDKAnnot| again because JS may have destroyed it - if (!pObservedAnnot) + GCedWidgetIteratorForAnnot(GetXFAPageView(), pSDKAnnot); + if (!pWidgetIterator) return nullptr; - if (pWidgetIterator->GetCurrentWidget() != pXFAWidget->GetXFAFFWidget()) - pWidgetIterator->SetCurrentWidget(pXFAWidget->GetXFAFFWidget()); - - CXFA_FFWidget* hNextFocus = - bNext ? pWidgetIterator->MoveToNext() : pWidgetIterator->MoveToPrevious(); - if (!hNextFocus) - return nullptr; - - return pPageView->GetAnnotByXFAWidget(hNextFocus); + return pSDKAnnot->GetPageView()->GetAnnotByXFAWidget( + pWidgetIterator->MoveToNext()); } -CPDFSDK_Annot* CPDFXFA_Page::GetFirstOrLastXFAAnnot(CPDFSDK_PageView* page_view, - bool last) const { - CXFA_FFPageView* xfa_page_view = GetXFAPageView(); - if (!xfa_page_view) +CPDFSDK_Annot* CPDFXFA_Page::GetPrevXFAAnnot(CPDFSDK_Annot* pSDKAnnot) const { + IXFA_WidgetIterator* pWidgetIterator = + GCedWidgetIteratorForAnnot(GetXFAPageView(), pSDKAnnot); + if (!pWidgetIterator) return nullptr; - ObservedPtr<CPDFSDK_PageView> watched_page_view(page_view); - IXFA_WidgetIterator* it = - xfa_page_view->CreateGCedTraverseWidgetIterator(kIteratorFilter); - if (!watched_page_view) + return pSDKAnnot->GetPageView()->GetAnnotByXFAWidget( + pWidgetIterator->MoveToPrevious()); +} + +CPDFSDK_Annot* CPDFXFA_Page::GetFirstXFAAnnot( + CPDFSDK_PageView* page_view) const { + IXFA_WidgetIterator* pWidgetIterator = + GCedWidgetIteratorForPage(GetXFAPageView(), page_view); + if (!pWidgetIterator) return nullptr; - CXFA_FFWidget* pWidget = last ? it->MoveToLast() : it->MoveToFirst(); - return watched_page_view->GetAnnotByXFAWidget(pWidget); + return page_view->GetAnnotByXFAWidget(pWidgetIterator->MoveToFirst()); +} + +CPDFSDK_Annot* CPDFXFA_Page::GetLastXFAAnnot( + CPDFSDK_PageView* page_view) const { + IXFA_WidgetIterator* pWidgetIterator = + GCedWidgetIteratorForPage(GetXFAPageView(), page_view); + if (!pWidgetIterator) + return nullptr; + + return page_view->GetAnnotByXFAWidget(pWidgetIterator->MoveToLast()); } int CPDFXFA_Page::HasFormFieldAtPoint(const CFX_PointF& point) const {
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.h b/fpdfsdk/fpdfxfa/cpdfxfa_page.h index b7655ef..14fafa0 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.h +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.h
@@ -47,9 +47,10 @@ int GetPageIndex() const { return m_iPageIndex; } 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; + CPDFSDK_Annot* GetNextXFAAnnot(CPDFSDK_Annot* pSDKAnnot) const; + CPDFSDK_Annot* GetPrevXFAAnnot(CPDFSDK_Annot* pSDKAnnot) const; + CPDFSDK_Annot* GetFirstXFAAnnot(CPDFSDK_PageView* page_view) const; + CPDFSDK_Annot* GetLastXFAAnnot(CPDFSDK_PageView* page_view) const; int HasFormFieldAtPoint(const CFX_PointF& point) const; void DrawFocusAnnot(CFX_RenderDevice* pDevice, CPDFSDK_Annot* pAnnot,