tdf#118967 Batch all a11y notifications

Currently all a11y notifications get send out immediately, which often
ends up formatting the document before it is ready. With the current
EnterBlockNotifications()/LeaveBlockNotifications() system it is difficult
to find all places that need blocking and any change in the a11y code
might require additional blocking in unpredictable places. By queueing
all notifications by default and only sending them out when the document
is ready, we can make sure that it can't be corrupted.

Change-Id: I9599c7b57eb5b8f8f0575de57fcc8bab171f78ff
Reviewed-on: https://gerrit.libreoffice.org/58703
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

editeng/source/editeng/editeng.cxx

@@ -1024,14 +1024,6 @@ bool EditEngine::PostKeyEvent( const KeyEvent& rKeyEvent, EditView* pEditView, v
         }
     }
 
-    pImpEditEngine->EnterBlockNotifications();
-
-    if ( GetNotifyHdl().IsSet() )
-    {
-        EENotify aNotify( EE_NOTIFY_INPUT_START );
-        pImpEditEngine->CallNotify( aNotify );
-    }
-
     if ( eFunc == KeyFuncType::DONTKNOW )
     {
         switch ( nCode )
@@ -1430,14 +1422,6 @@ bool EditEngine::PostKeyEvent( const KeyEvent& rKeyEvent, EditView* pEditView, v
         pImpEditEngine->CallStatusHdl();
     }
 
-    if ( GetNotifyHdl().IsSet() )
-    {
-        EENotify aNotify( EE_NOTIFY_INPUT_END );
-        pImpEditEngine->CallNotify( aNotify );
-    }
-
-    pImpEditEngine->LeaveBlockNotifications();
-
     return bDone;
 }
 
@@ -1534,10 +1518,8 @@ std::unique_ptr<EditTextObject> EditEngine::GetEmptyTextObject() const
 
 void EditEngine::SetText( const EditTextObject& rTextObject )
 {
-    pImpEditEngine->EnterBlockNotifications();
     pImpEditEngine->SetText( rTextObject );
     pImpEditEngine->FormatAndUpdate();
-    pImpEditEngine->LeaveBlockNotifications();
 }
 
 void EditEngine::ShowParagraph( sal_Int32 nParagraph, bool bShow )
@@ -2500,7 +2482,7 @@ void EditEngine::ParagraphInserted( sal_Int32 nPara )
     {
         EENotify aNotify( EE_NOTIFY_PARAGRAPHINSERTED );
         aNotify.nParagraph = nPara;
-        pImpEditEngine->CallNotify( aNotify );
+        pImpEditEngine->QueueNotify( aNotify );
     }
 }
 
@@ -2511,7 +2493,7 @@ void EditEngine::ParagraphDeleted( sal_Int32 nPara )
     {
         EENotify aNotify( EE_NOTIFY_PARAGRAPHREMOVED );
         aNotify.nParagraph = nPara;
-        pImpEditEngine->CallNotify( aNotify );
+        pImpEditEngine->QueueNotify( aNotify );
     }
 }
 void EditEngine::ParagraphConnected( sal_Int32 /*nLeftParagraph*/, sal_Int32 /*nRightParagraph*/ )
@@ -2533,7 +2515,7 @@ void EditEngine::ParagraphHeightChanged( sal_Int32 nPara )
     {
         EENotify aNotify( EE_NOTIFY_TextHeightChanged );
         aNotify.nParagraph = nPara;
-        pImpEditEngine->CallNotify( aNotify );
+        pImpEditEngine->QueueNotify( aNotify );
     }
 }
 

editeng/source/editeng/impedit.cxx

@@ -148,8 +148,10 @@ void ImpEditView::SetEditSelection( const EditSelection& rEditSelection )
             eNotifyType = EE_NOTIFY_TEXTVIEWSELECTIONCHANGED;
         }
         EENotify aNotify( eNotifyType );
-        pEditEngine->pImpEditEngine->CallNotify( aNotify );
+        pEditEngine->pImpEditEngine->QueueNotify( aNotify );
     }
+    if(pEditEngine->pImpEditEngine->IsFormatted())
+        pEditEngine->pImpEditEngine->SendNotifications();
 }
 
 /// Translate absolute <-> relative twips: LOK wants absolute coordinates as output and gives absolute coordinates as input.
@@ -1272,7 +1274,7 @@ Pair ImpEditView::Scroll( long ndX, long ndY, ScrollRangeCheck nRangeCheck )
         if ( pEditEngine->pImpEditEngine->GetNotifyHdl().IsSet() )
         {
             EENotify aNotify( EE_NOTIFY_TEXTVIEWSCROLLED );
-            pEditEngine->pImpEditEngine->CallNotify( aNotify );
+            pEditEngine->pImpEditEngine->QueueNotify( aNotify );
         }
     }
 
@@ -1563,11 +1565,9 @@ void ImpEditView::CutCopy( css::uno::Reference< css::datatransfer::clipboard::XC
 
         if (bCut)
         {
-            pEditEngine->pImpEditEngine->EnterBlockNotifications();
             pEditEngine->pImpEditEngine->UndoActionStart(EDITUNDO_CUT);
             DeleteSelected();
             pEditEngine->pImpEditEngine->UndoActionEnd();
-            pEditEngine->pImpEditEngine->LeaveBlockNotifications();
         }
     }
 }
@@ -1630,11 +1630,9 @@ void ImpEditView::Paste( css::uno::Reference< css::datatransfer::clipboard::XCli
         // Prevent notifications of paragraph inserts et al that would trigger
         // a11y to format content in a half-ready state when obtaining
         // paragraphs. Collect and broadcast when done instead.
-        pEditEngine->pImpEditEngine->EnterBlockNotifications();
         aSel = pEditEngine->InsertText(
             xDataObj, OUString(), aSel.Min(),
             bUseSpecial && pEditEngine->GetInternalEditStatus().AllowPasteSpecial());
-        pEditEngine->pImpEditEngine->LeaveBlockNotifications();
     }
 
     aPasteOrDropInfos.nEndPara = pEditEngine->GetEditDoc().GetPos( aSel.Max().GetNode() );

editeng/source/editeng/impedit.hxx

@@ -464,7 +464,6 @@ private:
 
     Color               maBackgroundColor;
 
-    sal_uInt32          nBlockNotifications;
     sal_uInt16          nStretchX;
     sal_uInt16          nStretchY;
 
@@ -791,14 +790,7 @@ public:
 
     void                EnableUndo( bool bEnable );
     bool                IsUndoEnabled()         { return bUndoEnabled; }
-    void                SetUndoMode( bool b )
-    {
-        bIsInUndo = b;
-        if (bIsInUndo)
-            EnterBlockNotifications();
-        else
-            LeaveBlockNotifications();
-    }
+    void                SetUndoMode( bool b )   { bIsInUndo = b; }
     bool                IsInUndo()              { return bIsInUndo; }
 
     void                SetCallParaInsertedOrDeleted( bool b ) { bCallParaInsertedOrDeleted = b; }
@@ -923,9 +915,8 @@ public:
     void                CallStatusHdl();
     void                DelayedCallStatusHdl()  { aStatusTimer.Start(); }
 
-    void                CallNotify( EENotify& rNotify );
-    void                EnterBlockNotifications();
-    void                LeaveBlockNotifications();
+    void                QueueNotify( EENotify& rNotify );
+    void                SendNotifications();
 
     void                UndoActionStart( sal_uInt16 nId );
     void                UndoActionStart( sal_uInt16 nId, const ESelection& rSel );

editeng/source/editeng/impedit2.cxx

@@ -121,7 +121,6 @@ ImpEditEngine::ImpEditEngine( EditEngine* pEE, SfxItemPool* pItemPool ) :
 
     nCurTextHeight      = 0;
     nCurTextHeightNTP   = 0;
-    nBlockNotifications = 0;
     nBigTextObjectStart = 20;
 
     nStretchX           = 100;
@@ -175,6 +174,18 @@ void ImpEditEngine::Dispose()
     pSharedVCL.reset();
 }
 
+void ImpEditEngine::SendNotifications()
+{
+    while(!aNotifyCache.empty())
+    {
+        GetNotifyHdl().Call( aNotifyCache[0] );
+        aNotifyCache.erase(aNotifyCache.begin());
+    }
+
+    EENotify aNotify(EE_NOTIFY_PROCESSNOTIFICATIONS);
+    GetNotifyHdl().Call(aNotify);
+}
+
 ImpEditEngine::~ImpEditEngine()
 {
     aStatusTimer.Stop();
@@ -625,9 +636,7 @@ bool ImpEditEngine::MouseMove( const MouseEvent& rMEvt, EditView* pView )
 
 EditPaM ImpEditEngine::InsertText(const EditSelection& aSel, const OUString& rStr)
 {
-    EnterBlockNotifications();
     EditPaM aPaM = ImpInsertText( aSel, rStr );
-    LeaveBlockNotifications();
     return aPaM;
 }
 
@@ -732,7 +741,7 @@ void ImpEditEngine::TextModified()
     if ( GetNotifyHdl().IsSet() )
     {
         EENotify aNotify( EE_NOTIFY_TEXTMODIFIED );
-        CallNotify( aNotify );
+        QueueNotify( aNotify );
     }
 }
 
@@ -2200,7 +2209,7 @@ EditSelection ImpEditEngine::ImpMoveParagraphs( Range aOldPositions, sal_Int32 n
         aNotify.nParagraph = nNewPos;
         aNotify.nParam1 = aOldPositions.Min();
         aNotify.nParam2 = aOldPositions.Max();
-        CallNotify( aNotify );
+        QueueNotify( aNotify );
     }
 
     aEditDoc.SetModified( true );
@@ -3406,7 +3415,6 @@ void ImpEditEngine::UpdateSelections()
             }
         }
     }
-
     aDeletedNodes.clear();
 }
 
@@ -4392,47 +4400,9 @@ bool ImpEditEngine::DoVisualCursorTraveling()
 }
 
 
-void ImpEditEngine::CallNotify( EENotify& rNotify )
-{
-    if ( !nBlockNotifications )
-        GetNotifyHdl().Call( rNotify );
-    else
-        aNotifyCache.push_back(rNotify);
-}
-
-void ImpEditEngine::EnterBlockNotifications()
+void ImpEditEngine::QueueNotify( EENotify& rNotify )
 {
-    if( !nBlockNotifications )
-    {
-        // #109864# Send out START notification immediately, to allow
-        // external, non-queued events to be captured as well from
-        // client side
-        EENotify aNotify( EE_NOTIFY_BLOCKNOTIFICATION_START );
-        GetNotifyHdl().Call( aNotify );
-    }
-
-    nBlockNotifications++;
-}
-
-void ImpEditEngine::LeaveBlockNotifications()
-{
-    OSL_ENSURE( nBlockNotifications, "LeaveBlockNotifications - Why?" );
-
-    nBlockNotifications--;
-    if ( !nBlockNotifications )
-    {
-        // Call blocked notify events...
-        while(!aNotifyCache.empty())
-        {
-            EENotify aNotify(aNotifyCache[0]);
-            // Remove from list before calling, maybe we enter LeaveBlockNotifications while calling the handler...
-            aNotifyCache.erase(aNotifyCache.begin());
-            GetNotifyHdl().Call( aNotify );
-        }
-
-        EENotify aNotify( EE_NOTIFY_BLOCKNOTIFICATION_END );
-        GetNotifyHdl().Call( aNotify );
-    }
+    aNotifyCache.push_back(rNotify);
 }
 
 IMPL_LINK_NOARG(ImpEditEngine, DocModified, LinkParamNone*, void)

editeng/source/editeng/impedit3.cxx

@@ -358,8 +358,6 @@ void ImpEditEngine::FormatDoc()
     if (!GetUpdateMode() || IsFormatting())
         return;
 
-    EnterBlockNotifications();
-
     bIsFormatting = true;
 
     // Then I can also start the spell-timer ...
@@ -474,8 +472,6 @@ void ImpEditEngine::FormatDoc()
         GetRefDevice()->Pop();
 
     CallStatusHdl();    // If Modified...
-
-    LeaveBlockNotifications();
 }
 
 bool ImpEditEngine::ImpCheckRefMapMode()
@@ -4241,6 +4237,8 @@ void ImpEditEngine::FormatAndUpdate( EditView* pCurView, bool bCalledFromUndo )
         FormatDoc();
         UpdateViews( pCurView );
     }
+
+    SendNotifications();
 }
 
 void ImpEditEngine::SetFlatMode( bool bFlat )

editeng/source/editeng/impedit4.cxx

@@ -1159,12 +1159,10 @@ void ImpEditEngine::SetText( const EditTextObject& rTextObject )
 
 EditSelection ImpEditEngine::InsertText( const EditTextObject& rTextObject, EditSelection aSel )
 {
-    EnterBlockNotifications();
     aSel.Adjust( aEditDoc );
     if ( aSel.HasRange() )
         aSel = ImpDeleteSelection( aSel );
     EditSelection aNewSel = InsertTextObject( rTextObject, aSel.Max() );
-    LeaveBlockNotifications();
     return aNewSel;
 }
 

editeng/source/uno/unoedhlp.cxx

@@ -69,17 +69,9 @@ std::unique_ptr<SfxHint> SvxEditSourceHelper::EENotification2Hint( EENotify cons
             case EE_NOTIFY_TEXTVIEWSELECTIONCHANGED:
                 return std::unique_ptr<SfxHint>( new SvxEditSourceHint( SfxHintId::EditSourceSelectionChanged ) );
 
-            case EE_NOTIFY_BLOCKNOTIFICATION_START:
-                return std::unique_ptr<SfxHint>( new TextHint( SfxHintId::TextBlockNotificationStart, 0 ) );
+            case EE_NOTIFY_PROCESSNOTIFICATIONS:
+                return std::unique_ptr<SfxHint>( new TextHint( SfxHintId::TextProcessNotifications ));
 
-            case EE_NOTIFY_BLOCKNOTIFICATION_END:
-                return std::unique_ptr<SfxHint>( new TextHint( SfxHintId::TextBlockNotificationEnd, 0 ) );
-
-            case EE_NOTIFY_INPUT_START:
-                return std::unique_ptr<SfxHint>( new TextHint( SfxHintId::TextInputStart, 0 ) );
-
-            case EE_NOTIFY_INPUT_END:
-                return std::unique_ptr<SfxHint>( new TextHint( SfxHintId::TextInputEnd, 0 ) );
             case EE_NOTIFY_TEXTVIEWSELECTIONCHANGED_ENDD_PARA:
                 return std::unique_ptr<SfxHint>( new SvxEditSourceHintEndPara );
             default:

include/editeng/editdata.hxx

@@ -326,23 +326,9 @@ enum EENotifyType
     /// The selection and/or the cursor position has changed
     EE_NOTIFY_TEXTVIEWSELECTIONCHANGED,
 
-    /** Denotes the beginning of a collected amount of EditEngine
-        notification events. This event itself is not queued, but sent
-        immediately
-     */
-    EE_NOTIFY_BLOCKNOTIFICATION_START,
-
-    /** Denotes the end of a collected amount of EditEngine
-        notification events. After this event, the queue is empty, and
-        a high-level operation such as "insert paragraph" is finished
-     */
-    EE_NOTIFY_BLOCKNOTIFICATION_END,
-
-    /// Denotes the beginning of a high-level action triggered by a key press
-    EE_NOTIFY_INPUT_START,
-
-    /// Denotes the end of a high-level action triggered by a key press
-    EE_NOTIFY_INPUT_END,
+    /// The EditEngine is in a valid state again. Process pending notifications.
+    EE_NOTIFY_PROCESSNOTIFICATIONS,
+
     EE_NOTIFY_TEXTVIEWSELECTIONCHANGED_ENDD_PARA
 };
 

include/svl/hint.hxx

@@ -47,10 +47,7 @@ enum class SfxHintId {
     TextFormatPara,
     TextFormatted,
     TextModified,
-    TextBlockNotificationStart,
-    TextBlockNotificationEnd,
-    TextInputStart,
-    TextInputEnd,
+    TextProcessNotifications,
     TextViewScrolled,
     TextViewSelectionChanged,
     TextViewCaretChanged,
@@ -142,10 +139,7 @@ inline std::basic_ostream<charT, traits> & operator <<(
     case SfxHintId::TextFormatPara: return stream << "TextFormatPara";
     case SfxHintId::TextFormatted: return stream << "TextFormatted";
     case SfxHintId::TextModified: return stream << "TextModified";
-    case SfxHintId::TextBlockNotificationStart: return stream << "TextBlockNotificationStart";
-    case SfxHintId::TextBlockNotificationEnd: return stream << "TextBlockNotificationEnd";
-    case SfxHintId::TextInputStart: return stream << "TextInputStart";
-    case SfxHintId::TextInputEnd: return stream << "TextInputEnd";
+    case SfxHintId::TextProcessNotifications: return stream << "TextProcessNotifications";
     case SfxHintId::TextViewScrolled: return stream << "TextViewScrolled";
     case SfxHintId::TextViewSelectionChanged: return stream << "TextViewSelectionChanged";
     case SfxHintId::TextViewCaretChanged: return stream << "TextViewCaretChanged";

svx/source/accessibility/AccessibleTextHelper.cxx

@@ -214,9 +214,6 @@ namespace accessibility
         // the object handling our children (guarded by solar mutex)
         ::accessibility::AccessibleParaManager maParaManager;
 
-        // number of not-yet-closed event frames (BEGIN/END sequences) (guarded by solar mutex)
-        sal_Int32 maEventOpenFrames;
-
         // Queued events from Notify() (guarded by solar mutex)
         AccessibleTextEventQueue maEventQueue;
 
@@ -244,7 +241,6 @@ namespace accessibility
         mnFirstVisibleChild( -1 ),
         mnLastVisibleChild( -2 ),
         mnStartIndex( 0 ),
-        maEventOpenFrames( 0 ),
         mbInNotify( false ),
         mbGroupHasFocus( false ),
         mbThisHasFocus( false ),
@@ -1312,77 +1308,32 @@ namespace accessibility
             // before that.
             if( const SvxViewChangedHint* pViewHint = dynamic_cast<const SvxViewChangedHint*>( &rHint ) )
             {
-                maEventQueue.Append( *pViewHint );
-
                 // process visibility right away, if not within an
                 // open EE notification frame. Otherwise, event
                 // processing would be delayed until next EE
                 // notification sequence.
-                if( maEventOpenFrames == 0 )
-                    ProcessQueue();
+                maEventQueue.Append( *pViewHint );
             }
             else if( const SdrHint* pSdrHint = dynamic_cast<const SdrHint*>( &rHint ) )
             {
-                maEventQueue.Append( *pSdrHint );
-
                 // process drawing layer events right away, if not
                 // within an open EE notification frame. Otherwise,
                 // event processing would be delayed until next EE
                 // notification sequence.
-                if( maEventOpenFrames == 0 )
-                    ProcessQueue();
+                maEventQueue.Append( *pSdrHint );
             }
             else if( const SvxEditSourceHint* pEditSourceHint = dynamic_cast<const SvxEditSourceHint*>( &rHint ) )
             {
-                maEventQueue.Append( *pEditSourceHint );
                 // EditEngine should emit TEXT_SELECTION_CHANGED events (#i27299#)
-                if( maEventOpenFrames == 0 )
-                    ProcessQueue();
+                maEventQueue.Append( *pEditSourceHint );
             }
             else if( const TextHint* pTextHint = dynamic_cast<const TextHint*>( &rHint ) )
             {
-                switch( pTextHint->GetId() )
-                {
-                    case SfxHintId::TextBlockNotificationEnd:
-                    case SfxHintId::TextInputEnd:
-                        --maEventOpenFrames;
-
-                        if( maEventOpenFrames == 0 )
-                        {
-                            /* All information should have arrived
-                             * now, process queue. As stated in the
-                             * above bug, we can often avoid throwing
-                             * away all paragraphs by looking forward
-                             * in the event queue (searching for
-                             * PARAINSERT/REMOVE events). Furthermore,
-                             * processing the event queue only at the
-                             * end of an interaction cycle, ensures
-                             * that the EditEngine state and the
-                             * AccessibleText state are the same
-                             * (well, mostly. If there are _multiple_
-                             * interaction cycles in the EE queues, it
-                             * can still happen that EE state is
-                             * different. That's so to say broken by
-                             * design with that delayed EE event
-                             * concept).
-                             */
-                            ProcessQueue();
-                        }
-                        break;
-
-                    case SfxHintId::TextBlockNotificationStart:
-                    case SfxHintId::TextInputStart:
-                        ++maEventOpenFrames;
-                        // no FALLTHROUGH reason: event will not be processed,
-                        // thus appending the event isn't necessary. (#i27299#)
-                        break;
-                    default:
-                        maEventQueue.Append( *pTextHint );
-                        // EditEngine should emit TEXT_SELECTION_CHANGED events (#i27299#)
-                        if( maEventOpenFrames == 0 )
-                            ProcessQueue();
-                        break;
-                }
+                // EditEngine should emit TEXT_SELECTION_CHANGED events (#i27299#)
+                if(pTextHint->GetId() == SfxHintId::TextProcessNotifications)
+                    ProcessQueue();
+                else
+                    maEventQueue.Append( *pTextHint );
             }
             // it's VITAL to keep the SfxHint last! It's the base of the classes above!
             else if( rHint.GetId() == SfxHintId::Dying )