editeng/source/accessibility/AccessibleParaManager.cxx | 17 - include/editeng/AccessibleParaManager.hxx | 1 svx/source/accessibility/AccessibleTextEventQueue.cxx | 1 svx/source/accessibility/AccessibleTextEventQueue.hxx | 4 svx/source/accessibility/AccessibleTextHelper.cxx | 246 ++++++++--------- 5 files changed, 130 insertions(+), 139 deletions(-)
New commits: commit 52156459aa72ebc098e16e89ceccc0e573f03995 Author: Michael Weghorn <[email protected]> AuthorDate: Fri Aug 15 12:32:59 2025 +0200 Commit: Michael Weghorn <[email protected]> CommitDate: Fri Aug 15 17:41:39 2025 +0200 svx a11y: Switch check to assert in AccessibleTextHelper_Impl::ProcessQueue AccessibleTextHelper_Impl::maEventQueue should never contain any null entries. Add a corresponding assert and drop the if check. Also add an assert that AccessibleTextEventQueue::PopFront never gets called when it doesn't contain any entries. (`git show --ignore-space-change` helps to see the "actual change" more clearly.) Change-Id: Ide8effa8795c95f71adc2220b34c3aec6403b66a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/189679 Reviewed-by: Michael Weghorn <[email protected]> Tested-by: Jenkins diff --git a/svx/source/accessibility/AccessibleTextEventQueue.cxx b/svx/source/accessibility/AccessibleTextEventQueue.cxx index a39123c45eca..272f71c89d97 100644 --- a/svx/source/accessibility/AccessibleTextEventQueue.cxx +++ b/svx/source/accessibility/AccessibleTextEventQueue.cxx @@ -68,6 +68,7 @@ namespace accessibility ::std::unique_ptr< SfxHint > AccessibleTextEventQueue::PopFront() { + assert(!maEventQueue.empty()); ::std::unique_ptr< SfxHint > aRes( *(maEventQueue.begin()) ); maEventQueue.pop_front(); return aRes; diff --git a/svx/source/accessibility/AccessibleTextHelper.cxx b/svx/source/accessibility/AccessibleTextHelper.cxx index c8eb9180d9a7..339331011192 100644 --- a/svx/source/accessibility/AccessibleTextHelper.cxx +++ b/svx/source/accessibility/AccessibleTextHelper.cxx @@ -1117,171 +1117,169 @@ void AccessibleTextHelper_Impl::ProcessQueue() while( !maEventQueue.IsEmpty() ) { ::std::unique_ptr< SfxHint > pHint( maEventQueue.PopFront() ); - if (pHint) - { - const SfxHint& rHint = *pHint; + assert(pHint); + const SfxHint& rHint = *pHint; - // Note, if you add events here, you need to update the AccessibleTextEventQueue::Append - // code, because only the events we process here, are actually queued there. + // Note, if you add events here, you need to update the AccessibleTextEventQueue::Append + // code, because only the events we process here, are actually queued there. - try + try + { + + if (rHint.GetId() == SfxHintId::ThisIsAnSdrHint) { + const SdrHint* pSdrHint = static_cast< const SdrHint* >( &rHint ); - if (rHint.GetId() == SfxHintId::ThisIsAnSdrHint) + switch( pSdrHint->GetKind() ) { - const SdrHint* pSdrHint = static_cast< const SdrHint* >( &rHint ); - - switch( pSdrHint->GetKind() ) + case SdrHintKind::BeginEdit: { - case SdrHintKind::BeginEdit: + if(!IsActive()) { - if(!IsActive()) - { - break; - } - // change children state - maParaManager.SetActive(); - - // per definition, edit mode text has the focus - SetFocus( true ); break; } + // change children state + maParaManager.SetActive(); - case SdrHintKind::EndEdit: - { - // focused child now loses focus - ESelection aSelection; - if( GetEditViewForwarder().GetSelection( aSelection ) ) - SetChildFocus(aSelection.end.nPara, false); + // per definition, edit mode text has the focus + SetFocus( true ); + break; + } - // change children state - maParaManager.SetActive( false ); + case SdrHintKind::EndEdit: + { + // focused child now loses focus + ESelection aSelection; + if( GetEditViewForwarder().GetSelection( aSelection ) ) + SetChildFocus(aSelection.end.nPara, false); - maLastSelection = ESelection::AtEnd(); - break; - } - default: - break; + // change children state + maParaManager.SetActive( false ); + + maLastSelection = ESelection::AtEnd(); + break; } + default: + break; } - else if( const SvxEditSourceHint* pEditSourceHint = dynamic_cast<const SvxEditSourceHint*>( &rHint ) ) + } + else if( const SvxEditSourceHint* pEditSourceHint = dynamic_cast<const SvxEditSourceHint*>( &rHint ) ) + { + switch( pEditSourceHint->GetId() ) { - switch( pEditSourceHint->GetId() ) + case SfxHintId::EditSourceParasMoved: { - case SfxHintId::EditSourceParasMoved: - { - DBG_ASSERT( pEditSourceHint->GetStartValue() < GetTextForwarder().GetParagraphCount() && - pEditSourceHint->GetEndValue() < GetTextForwarder().GetParagraphCount(), - "AccessibleTextHelper_Impl::NotifyHdl: Invalid notification"); + DBG_ASSERT( pEditSourceHint->GetStartValue() < GetTextForwarder().GetParagraphCount() && + pEditSourceHint->GetEndValue() < GetTextForwarder().GetParagraphCount(), + "AccessibleTextHelper_Impl::NotifyHdl: Invalid notification"); - if( !bEverythingUpdated ) - { - ParagraphsMoved(pEditSourceHint->GetStartValue(), - pEditSourceHint->GetValue(), - pEditSourceHint->GetEndValue()); + if( !bEverythingUpdated ) + { + ParagraphsMoved(pEditSourceHint->GetStartValue(), + pEditSourceHint->GetValue(), + pEditSourceHint->GetEndValue()); - // in all cases, check visibility afterwards. - UpdateVisibleChildren(); - } - break; + // in all cases, check visibility afterwards. + UpdateVisibleChildren(); } - - case SfxHintId::EditSourceSelectionChanged: - // notify listeners - try - { - UpdateSelection(); - } - // maybe we're not in edit mode (this is not an error) - catch( const uno::Exception& ) {} - break; - default: break; + break; } + + case SfxHintId::EditSourceSelectionChanged: + // notify listeners + try + { + UpdateSelection(); + } + // maybe we're not in edit mode (this is not an error) + catch( const uno::Exception& ) {} + break; + default: break; } - else if( const TextHint* pTextHint = dynamic_cast<const TextHint*>( &rHint ) ) - { - const sal_Int32 nParas = GetTextForwarder().GetParagraphCount(); + } + else if( const TextHint* pTextHint = dynamic_cast<const TextHint*>( &rHint ) ) + { + const sal_Int32 nParas = GetTextForwarder().GetParagraphCount(); - switch( pTextHint->GetId() ) + switch( pTextHint->GetId() ) + { + case SfxHintId::TextModified: { - case SfxHintId::TextModified: - { - // notify listeners - sal_Int32 nPara( pTextHint->GetValue() ); + // notify listeners + sal_Int32 nPara( pTextHint->GetValue() ); - // #108900# Delegate change event to children - AccessibleTextHelper_ChildrenTextChanged aNotifyChildrenFunctor; + // #108900# Delegate change event to children + AccessibleTextHelper_ChildrenTextChanged aNotifyChildrenFunctor; - if (nPara == EE_PARA_MAX) + if (nPara == EE_PARA_MAX) + { + // #108900# Call every child + ::std::for_each( maParaManager.begin(), maParaManager.end(), + AccessibleParaManager::WeakChildAdapter< AccessibleTextHelper_ChildrenTextChanged > (aNotifyChildrenFunctor) ); + } + else + if( nPara < nParas ) { - // #108900# Call every child - ::std::for_each( maParaManager.begin(), maParaManager.end(), + // #108900# Call child at index nPara + ::std::for_each( maParaManager.begin()+nPara, maParaManager.begin()+nPara+1, AccessibleParaManager::WeakChildAdapter< AccessibleTextHelper_ChildrenTextChanged > (aNotifyChildrenFunctor) ); } - else - if( nPara < nParas ) - { - // #108900# Call child at index nPara - ::std::for_each( maParaManager.begin()+nPara, maParaManager.begin()+nPara+1, - AccessibleParaManager::WeakChildAdapter< AccessibleTextHelper_ChildrenTextChanged > (aNotifyChildrenFunctor) ); - } - break; - } + break; + } - case SfxHintId::TextParaInserted: - // already happened above - break; + case SfxHintId::TextParaInserted: + // already happened above + break; - case SfxHintId::TextParaRemoved: - // already happened above - break; + case SfxHintId::TextParaRemoved: + // already happened above + break; - case SfxHintId::TextHeightChanged: - // visibility changed, done below - break; - - case SfxHintId::TextViewScrolled: - // visibility changed, done below - break; - default: break; - } + case SfxHintId::TextHeightChanged: + // visibility changed, done below + break; - // in all cases, check visibility afterwards. - if (!bUpdatedBoundRectAndVisibleChildren) - { - UpdateVisibleChildren(); - UpdateBoundRect(); - bUpdatedBoundRectAndVisibleChildren = true; - } + case SfxHintId::TextViewScrolled: + // visibility changed, done below + break; + default: break; } - else if (rHint.GetId() == SfxHintId::SvxViewChanged) + + // in all cases, check visibility afterwards. + if (!bUpdatedBoundRectAndVisibleChildren) { - // just check visibility - if (!bUpdatedBoundRectAndVisibleChildren) - { - UpdateVisibleChildren(); - UpdateBoundRect(); - bUpdatedBoundRectAndVisibleChildren = true; - } + UpdateVisibleChildren(); + UpdateBoundRect(); + bUpdatedBoundRectAndVisibleChildren = true; } - // it's VITAL to keep the SfxSimpleHint last! It's the base of some classes above! - else if( rHint.GetId() == SfxHintId::Dying) + } + else if (rHint.GetId() == SfxHintId::SvxViewChanged) + { + // just check visibility + if (!bUpdatedBoundRectAndVisibleChildren) { - // edit source is dying under us, become defunc then - try - { - // make edit source inaccessible - // Note: cannot destroy it here, since we're called from there! - ShutdownEditSource(); - } - catch( const uno::Exception& ) {} + UpdateVisibleChildren(); + UpdateBoundRect(); + bUpdatedBoundRectAndVisibleChildren = true; } } - catch( const uno::Exception& ) + // it's VITAL to keep the SfxSimpleHint last! It's the base of some classes above! + else if( rHint.GetId() == SfxHintId::Dying) { - DBG_UNHANDLED_EXCEPTION("svx"); + // edit source is dying under us, become defunc then + try + { + // make edit source inaccessible + // Note: cannot destroy it here, since we're called from there! + ShutdownEditSource(); + } + catch( const uno::Exception& ) {} } } + catch( const uno::Exception& ) + { + DBG_UNHANDLED_EXCEPTION("svx"); + } } } commit bd02e33c6c65bbb6fbc58662590febfb00e401aa Author: Michael Weghorn <[email protected]> AuthorDate: Fri Aug 15 12:20:03 2025 +0200 Commit: Michael Weghorn <[email protected]> CommitDate: Fri Aug 15 17:41:33 2025 +0200 svx a11y: Drop EventQueue typedef It is only used once. Change-Id: Idc5b0fcafdd06a3190d012d3efb546f3eadc94aa Reviewed-on: https://gerrit.libreoffice.org/c/core/+/189678 Tested-by: Jenkins Reviewed-by: Michael Weghorn <[email protected]> diff --git a/svx/source/accessibility/AccessibleTextEventQueue.hxx b/svx/source/accessibility/AccessibleTextEventQueue.hxx index 23dbf9faa648..b8b1a9de853a 100644 --- a/svx/source/accessibility/AccessibleTextEventQueue.hxx +++ b/svx/source/accessibility/AccessibleTextEventQueue.hxx @@ -41,8 +41,6 @@ namespace accessibility class AccessibleTextEventQueue { public: - typedef ::std::deque< SfxHint* > EventQueue; - AccessibleTextEventQueue(); ~AccessibleTextEventQueue(); @@ -80,7 +78,7 @@ namespace accessibility void Clear(); private: - EventQueue maEventQueue; + std::deque<SfxHint*> maEventQueue; }; } // end of namespace accessibility commit 2bb5079d39b0fa9a463397d08f67130eb9d4729a Author: Michael Weghorn <[email protected]> AuthorDate: Fri Aug 15 11:49:07 2025 +0200 Commit: Michael Weghorn <[email protected]> CommitDate: Fri Aug 15 17:41:26 2025 +0200 editeng a11y: Drop AccessibleParaManager::IsReferencable ... variant that only calls rtl::Reference<AccessibleEditableTextPara>::is(). Instead, spell out the type instead of using `auto` to make this a bit more readable/obvious in the callers. Change-Id: I53e2e2b8d22cd65d9d3d1f5b17e0c3ac17a711e7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/189677 Reviewed-by: Michael Weghorn <[email protected]> Tested-by: Jenkins diff --git a/editeng/source/accessibility/AccessibleParaManager.cxx b/editeng/source/accessibility/AccessibleParaManager.cxx index 959261ceea36..b5ebb056968e 100644 --- a/editeng/source/accessibility/AccessibleParaManager.cxx +++ b/editeng/source/accessibility/AccessibleParaManager.cxx @@ -105,12 +105,6 @@ void AccessibleParaManager::FireEvent( sal_Int32 nPara, } } -bool AccessibleParaManager::IsReferencable( - rtl::Reference<AccessibleEditableTextPara> const & aChild) -{ - return aChild.is(); -} - bool AccessibleParaManager::IsReferencable( sal_Int32 nChild ) const { assert(0 <= nChild && maChildren.size() > o3tl::make_unsigned(nChild) @@ -119,7 +113,8 @@ bool AccessibleParaManager::IsReferencable( sal_Int32 nChild ) const if( 0 <= nChild && maChildren.size() > o3tl::make_unsigned(nChild) ) { // retrieve hard reference from weak one - return IsReferencable( GetChild( nChild ).first.get() ); + rtl::Reference<AccessibleEditableTextPara> pChild = GetChild(nChild).first.get(); + return pChild.is(); } else { @@ -374,11 +369,11 @@ void AccessibleParaManager::Release( sal_Int32 nStartPara, sal_Int32 nEndPara ) std::transform(front, back, front, [](const AccessibleParaManager::WeakChild& rPara) { - auto aChild(rPara.first.get()); - if (IsReferencable(aChild)) + rtl::Reference<AccessibleEditableTextPara> pChild = rPara.first.get(); + if (pChild.is()) { - aChild->SetEditSource(nullptr); - aChild->dispose(); + pChild->SetEditSource(nullptr); + pChild->dispose(); } // clear reference diff --git a/include/editeng/AccessibleParaManager.hxx b/include/editeng/AccessibleParaManager.hxx index cd5ef8a48984..c2eab2cb17e1 100644 --- a/include/editeng/AccessibleParaManager.hxx +++ b/include/editeng/AccessibleParaManager.hxx @@ -81,7 +81,6 @@ public: void FireEvent( sal_Int32 nPara, const sal_Int16 nEventId ) const; - static bool IsReferencable(rtl::Reference<AccessibleEditableTextPara> const & aChild); bool IsReferencable( sal_Int32 nChild ) const; rtl::Reference<comphelper::OAccessible>
