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>

10 files changed, 43 insertions, 183 deletions

diff --git a/editeng/source/editeng/editeng.cxx b/editeng/source/editeng/editeng.cxx
index e601fa4172e7..40a1dcb5adfe 100644
--- a/editeng/source/editeng/editeng.cxx
+++ b/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 );
     }
 }
 
diff --git a/editeng/source/editeng/impedit.cxx b/editeng/source/editeng/impedit.cxx
index 7b8f9fbd112e..818722b34f57 100644
--- a/editeng/source/editeng/impedit.cxx
+++ b/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() );
diff --git a/editeng/source/editeng/impedit.hxx b/editeng/source/editeng/impedit.hxx
index c8199bd72045..08951792e149 100644
--- a/editeng/source/editeng/impedit.hxx
+++ b/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 );
diff --git a/editeng/source/editeng/impedit2.cxx b/editeng/source/editeng/impedit2.cxx
index 603290f9609b..f85c9138ef44 100644
--- a/editeng/source/editeng/impedit2.cxx
+++ b/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)
diff --git a/editeng/source/editeng/impedit3.cxx b/editeng/source/editeng/impedit3.cxx
index 6287609944bb..320707ecda59 100644
--- a/editeng/source/editeng/impedit3.cxx
+++ b/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 )
diff --git a/editeng/source/editeng/impedit4.cxx b/editeng/source/editeng/impedit4.cxx
index 5c2378917381..471301196295 100644
--- a/editeng/source/editeng/impedit4.cxx
+++ b/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;
 }
 
diff --git a/editeng/source/uno/unoedhlp.cxx b/editeng/source/uno/unoedhlp.cxx
index 1dffdc82da0f..79393490cf22 100644
--- a/editeng/source/uno/unoedhlp.cxx
+++ b/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:
diff --git a/include/editeng/editdata.hxx b/include/editeng/editdata.hxx
index ced6ab6287dd..0b2e464ca015 100644
--- a/include/editeng/editdata.hxx
+++ b/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
 };
 
diff --git a/include/svl/hint.hxx b/include/svl/hint.hxx
index cda13c11e313..9091e2de4336 100644
--- a/include/svl/hint.hxx
+++ b/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";
diff --git a/svx/source/accessibility/AccessibleTextHelper.cxx b/svx/source/accessibility/AccessibleTextHelper.cxx
index cf33d7dc7a7e..29f670dc8d8f 100644
--- a/svx/source/accessibility/AccessibleTextHelper.cxx
+++ b/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 )