tdf#151548 sw content controls: keyboard navigation with tab key

Combine content controls with legacy formfield controls
in keyboard tab navigation.

MS Word (I tested 2010) is extremely irrational and inconsistent
in its behaviour, so I modeled my implementation on the specification
and general logic, and not at all on "compatible misbehaviour".

There is a third category of form control (activeX rich content),
but these are mapped to internal LO controls that are only exposed
at VCL level, and don't pass the keystrokes back to SW.
Plus, they are not inline, but fly controls.

However, it is still a TODO to handle these if reasonably possible.

Change-Id: I1fef34d05a779e9d4f549987238435acb6c043d2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146219
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146563
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>

10 files changed, 211 insertions, 10 deletions

diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index 03c3dcfce648..93991cdd1987 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -326,6 +326,9 @@ class IDocumentMarkAccess
         */
         virtual const_iterator_t getFieldmarksEnd() const =0;
 
+        /// returns the number of IFieldmarks.
+        virtual sal_Int32 getFieldmarksCount() const = 0;
+
         /// get Fieldmark for CH_TXT_ATR_FIELDSTART/CH_TXT_ATR_FIELDEND at rPos
         virtual ::sw::mark::IFieldmark* getFieldmarkAt(const SwPosition& rPos) const =0;
         virtual ::sw::mark::IFieldmark* getFieldmarkFor(const SwPosition& pos) const =0;
diff --git a/sw/inc/crsrsh.hxx b/sw/inc/crsrsh.hxx
index 25e95c415fa4..431cdd503ab3 100644
--- a/sw/inc/crsrsh.hxx
+++ b/sw/inc/crsrsh.hxx
@@ -709,6 +709,8 @@ public:
 
     bool GotoFormatContentControl(const SwFormatContentControl& rContentControl);
 
+    void GotoFormControl(bool bNext);
+
     static SwTextField* GetTextFieldAtPos(
         const SwPosition* pPos,
         const bool bIncludeInputFieldAtStart );
diff --git a/sw/inc/textcontentcontrol.hxx b/sw/inc/textcontentcontrol.hxx
index 3fb7ea124b99..b3926bd25ce9 100644
--- a/sw/inc/textcontentcontrol.hxx
+++ b/sw/inc/textcontentcontrol.hxx
@@ -64,6 +64,7 @@ public:
     size_t GetCount() const { return m_aContentControls.size(); }
     bool IsEmpty() const { return m_aContentControls.empty(); }
     SwTextContentControl* Get(size_t nIndex);
+    SwTextContentControl* UnsortedGet(size_t nIndex);
     void dumpAsXml(xmlTextWriterPtr pWriter) const;
 };
 
diff --git a/sw/qa/extras/uiwriter/data/tdf151548_tabNavigation.docm b/sw/qa/extras/uiwriter/data/tdf151548_tabNavigation.docm
new file mode 100644
index 000000000000..1b173e2041c2
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data/tdf151548_tabNavigation.docm
diff --git a/sw/qa/extras/uiwriter/uiwriter4.cxx b/sw/qa/extras/uiwriter/uiwriter4.cxx
index 967ed5007522..715f58334002 100644
--- a/sw/qa/extras/uiwriter/uiwriter4.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter4.cxx
@@ -195,6 +195,7 @@ public:
     void testCursorWindows();
     void testLandscape();
     void testTdf95699();
+    void testTdf151548_tabNavigation();
     void testTdf104032();
     void testTdf104440();
     void testTdf104425();
@@ -324,6 +325,7 @@ public:
     CPPUNIT_TEST(testCursorWindows);
     CPPUNIT_TEST(testLandscape);
     CPPUNIT_TEST(testTdf95699);
+    CPPUNIT_TEST(testTdf151548_tabNavigation);
     CPPUNIT_TEST(testTdf104032);
     CPPUNIT_TEST(testTdf104440);
     CPPUNIT_TEST(testTdf104425);
@@ -1452,6 +1454,45 @@ void SwUiWriterTest4::testTdf95699()
                          pFieldMark->GetFieldname());
 }
 
+void SwUiWriterTest4::testTdf151548_tabNavigation()
+{
+    // given a form-protected doc with 4 unchecked legacy fieldmark checkboxes (and several modern
+    // content controls which all have a tabstop of -1 to disable tabstop navigation to them)
+    // we want to test that tab navigation completes and loops around to continue at the beginning.
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf151548_tabNavigation.docm");
+    SwXTextDocument* pXTextDocument = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+
+    IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess();
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMarkAccess->getFieldmarksCount());
+
+    // Tab and toggle 4 times, verifying beforehand that the state was unchecked
+    for (auto it = pMarkAccess->getFieldmarksBegin(); it != pMarkAccess->getFieldmarksEnd(); ++it)
+    {
+        sw::mark::ICheckboxFieldmark* pCheckBox
+            = dynamic_cast<::sw::mark::ICheckboxFieldmark*>(*it);
+        CPPUNIT_ASSERT(!pCheckBox->IsChecked());
+
+        pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 32, KEY_SPACE); // toggle checkbox on
+        pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, KEY_TAB); // move to next control
+        Scheduler::ProcessEventsToIdle();
+    }
+
+    // Tab 4 more times, verifying beforehand that the checkbox had been toggle on, then toggles off
+    // meaning that looping is working, and no other controls are reacting to the tab key.
+    for (auto it = pMarkAccess->getFieldmarksBegin(); it != pMarkAccess->getFieldmarksEnd(); ++it)
+    {
+        sw::mark::ICheckboxFieldmark* pCheckBox
+            = dynamic_cast<::sw::mark::ICheckboxFieldmark*>(*it);
+
+        CPPUNIT_ASSERT(pCheckBox->IsChecked());
+        pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 32, KEY_SPACE); // toggle checkbox off
+        Scheduler::ProcessEventsToIdle();
+
+        CPPUNIT_ASSERT(!pCheckBox->IsChecked());
+        pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, KEY_TAB); // move to next control
+    }
+}
+
 void SwUiWriterTest4::testTdf104032()
 {
     // Open the document with FORMCHECKBOX field, select it and copy to clipboard
diff --git a/sw/source/core/crsr/crstrvl.cxx b/sw/source/core/crsr/crstrvl.cxx
index 42f80e9bd013..74fe9b15ad57 100644
--- a/sw/source/core/crsr/crstrvl.cxx
+++ b/sw/source/core/crsr/crstrvl.cxx
@@ -894,6 +894,132 @@ bool SwCursorShell::GotoFormatContentControl(const SwFormatContentControl& rCont
     return bRet;
 }
 
+/**
+ * Go to the next (or previous) form control, based first on tabIndex and then paragraph position,
+ * where a tabIndex of 1 is first, 0 is last, and -1 is excluded.
+ */
+void SwCursorShell::GotoFormControl(bool bNext)
+{
+    // (note: this only applies to modern content controls and legacy fieldmarks,
+    //  since activeX richText controls aren't exposed to SW keystrokes)
+
+    struct FormControlSort
+    {
+        bool operator()(std::pair<const SwPosition&, sal_uInt32> rLHS,
+                        std::pair<const SwPosition&, sal_uInt32> rRHS) const
+        {
+            assert(rLHS.second && rRHS.second && "tabIndex zero must be changed to SAL_MAX_UINT32");
+            //first compare tabIndexes where 1 has the priority.
+            if (rLHS.second < rRHS.second)
+                return true;
+            if (rLHS.second > rRHS.second)
+                return false;
+
+            // when tabIndexes are equal (and they usually are) then sort by paragraph position
+            return rLHS.first < rRHS.first;
+        }
+    };
+    std::map<std::pair<SwPosition, sal_uInt32>,
+             std::pair<SwTextContentControl*, sw::mark::IFieldmark*>, FormControlSort>  aFormMap;
+
+    // add all of the eligible modern Content Controls into a sorted map
+    SwContentControlManager& rManager = GetDoc()->GetContentControlManager();
+    for (size_t  i = 0; i < rManager.GetCount(); ++i)
+    {
+        SwTextContentControl* pTCC = rManager.UnsortedGet(i);
+        if (!pTCC || !pTCC->GetTextNode())
+            continue;
+        auto pCC = pTCC->GetContentControl().GetContentControl();
+
+        // -1 indicates the control should not participate in keyboard tab navigation
+        if (pCC && pCC->GetTabIndex() == SAL_MAX_UINT32)
+            continue;
+
+        const SwPosition nPos(*pTCC->GetTextNode(), pTCC->GetStart());
+
+        // since 0 is the lowest priority (1 is the highest), and -1 has already been excluded,
+        // use SAL_MAX_UINT32 as zero's tabIndex so that automatic sorting is correct.
+        sal_uInt32 nTabIndex = pCC && pCC->GetTabIndex() ? pCC->GetTabIndex() : SAL_MAX_UINT32;
+
+        const std::pair<SwTextContentControl*, sw::mark::IFieldmark*> pFormControl(pTCC, nullptr);
+        aFormMap[std::make_pair(nPos, nTabIndex)] = pFormControl;
+    }
+
+    if (aFormMap.begin() == aFormMap.end())
+    {
+        // only legacy fields exist. Avoid reprocessing everything and use legacy code path.
+        GotoFieldmark(bNext ? GetFieldmarkAfter(/*Loop=*/true) : GetFieldmarkBefore(/*Loop=*/true));
+        return;
+    }
+
+    // add all of the legacy form field controls into the sorted map
+    IDocumentMarkAccess* pMarkAccess = GetDoc()->getIDocumentMarkAccess();
+    for (auto it = pMarkAccess->getFieldmarksBegin(); it != pMarkAccess->getFieldmarksEnd(); ++it)
+    {
+        auto pFieldMark = dynamic_cast<sw::mark::IFieldmark*>(*it);
+        assert(pFieldMark);
+        std::pair<SwTextContentControl*, sw::mark::IFieldmark*> pFormControl(nullptr, pFieldMark);
+        // legacy form fields do not have (functional) tabIndexes - use lowest priority for them
+        aFormMap[std::make_pair((*it)->GetMarkStart(), SAL_MAX_UINT32)] = pFormControl;
+    }
+
+    if (aFormMap.begin() == aFormMap.end())
+        return;
+
+    // Identify the current location in the document, and the current tab index priority
+
+    // A content control could contain a Fieldmark, so check for legacy fieldmarks first
+    sw::mark::IFieldmark* pFieldMark = GetCurrentFieldmark();
+    SwTextContentControl* pTCC = !pFieldMark ? CursorInsideContentControl() : nullptr;
+
+    auto pCC = pTCC ? pTCC->GetContentControl().GetContentControl() : nullptr;
+    const sal_Int32 nCurTabIndex = pCC && pCC->GetTabIndex() ? pCC->GetTabIndex() : SAL_MAX_UINT32;
+
+    SwPosition nCurPos(*GetCursor()->GetPoint());
+    if (pFieldMark)
+        nCurPos = pFieldMark->GetMarkStart();
+    else if (pTCC && pTCC->GetTextNode())
+        nCurPos = SwPosition(*pTCC->GetTextNode(), pTCC->GetStart());
+
+    // Find the previous (or next) tab control and navigate to it
+    const std::pair<SwPosition, sal_uInt32> nOldPos(nCurPos, nCurTabIndex);
+
+    // lower_bound acts like find, and returns a pointer to nFindPos if it exists,
+    // otherwise it will point to the previous entry.
+    auto aNewPos = aFormMap.lower_bound(nOldPos);
+    if (bNext && aNewPos != aFormMap.end())
+        ++aNewPos;
+    else if (!bNext && aNewPos != aFormMap.end() && aNewPos->first == nOldPos)
+    {
+        // Found the current position - need to return previous
+        if (aNewPos == aFormMap.begin())
+            aNewPos = aFormMap.end(); // prepare to loop around
+        else
+            --aNewPos;
+    }
+
+    if (aNewPos == aFormMap.end())
+    {
+        // Loop around to the other side
+        if (bNext)
+            aNewPos = aFormMap.begin();
+        else
+            --aNewPos;
+    }
+
+    // the entry contains a pointer to either a Content Control (first) or Fieldmark (second)
+    if (aNewPos->second.first)
+    {
+        auto& rFCC = static_cast<SwFormatContentControl&>(aNewPos->second.first->GetAttr());
+        GotoFormatContentControl(rFCC);
+    }
+    else
+    {
+        assert(aNewPos->second.second);
+        GotoFieldmark(aNewPos->second.second);
+    }
+}
+
 bool SwCursorShell::GotoFormatField( const SwFormatField& rField )
 {
     bool bRet = false;
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 19e229f45684..4b9a98615868 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1385,6 +1385,8 @@ namespace sw::mark
     IDocumentMarkAccess::const_iterator_t MarkManager::getFieldmarksEnd() const
         { return m_vFieldmarks.end(); }
 
+    sal_Int32 MarkManager::getFieldmarksCount() const { return m_vFieldmarks.size(); }
+
 
     // finds the first that is starting after
     IDocumentMarkAccess::const_iterator_t MarkManager::findFirstBookmarkStartsAfter(const SwPosition& rPos) const
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 214bcae02669..84f92e68a31e 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -89,6 +89,7 @@ namespace sw::mark {
             // Fieldmarks
             virtual const_iterator_t getFieldmarksBegin() const override;
             virtual const_iterator_t getFieldmarksEnd() const override;
+            virtual sal_Int32 getFieldmarksCount() const override;
             virtual ::sw::mark::IFieldmark* getFieldmarkAt(const SwPosition& rPos) const override;
             virtual ::sw::mark::IFieldmark* getFieldmarkFor(const SwPosition& rPos) const override;
             virtual sw::mark::IFieldmark* getFieldmarkBefore(const SwPosition& rPos, bool bLoop) const override;
diff --git a/sw/source/core/txtnode/attrcontentcontrol.cxx b/sw/source/core/txtnode/attrcontentcontrol.cxx
index a54d4ef83429..07e7afceb027 100644
--- a/sw/source/core/txtnode/attrcontentcontrol.cxx
+++ b/sw/source/core/txtnode/attrcontentcontrol.cxx
@@ -803,6 +803,12 @@ SwTextContentControl* SwContentControlManager::Get(size_t nIndex)
     return m_aContentControls[nIndex];
 }
 
+SwTextContentControl* SwContentControlManager::UnsortedGet(size_t nIndex)
+{
+    assert(nIndex < m_aContentControls.size());
+    return m_aContentControls[nIndex];
+}
+
 void SwContentControlManager::dumpAsXml(xmlTextWriterPtr pWriter) const
 {
     (void)xmlTextWriterStartElement(pWriter, BAD_CAST("SwContentControlManager"));
diff --git a/sw/source/uibase/docvw/edtwin.cxx b/sw/source/uibase/docvw/edtwin.cxx
index 4eede8fb0755..615278a769b3 100644
--- a/sw/source/uibase/docvw/edtwin.cxx
+++ b/sw/source/uibase/docvw/edtwin.cxx
@@ -2071,8 +2071,11 @@ KEYINPUT_CHECKTABLE_INSDEL:
                     }
                 case KEY_TAB:
                 {
-
-                    if (rSh.IsFormProtected() || rSh.GetCurrentFieldmark() || rSh.GetChar(false)==CH_TXT_ATR_FORMELEMENT)
+                    // Rich text contentControls accept tabs and fieldmarks and other rich text,
+                    // so first act on cases that are not a content control
+                    SwTextContentControl* pTextContentControl = rSh.CursorInsideContentControl();
+                    if ((rSh.IsFormProtected() && !pTextContentControl) ||
+                        rSh.GetCurrentFieldmark() || rSh.GetChar(false)==CH_TXT_ATR_FORMELEMENT)
                     {
                         eKeyState = SwKeyState::GotoNextFieldMark;
                     }
@@ -2120,6 +2123,21 @@ KEYINPUT_CHECKTABLE_INSDEL:
                             eNextKeyState = SwKeyState::NextCell;
                         }
                     }
+                    else if (pTextContentControl)
+                    {
+                        auto pCC = pTextContentControl->GetContentControl().GetContentControl();
+                        if (pCC)
+                        {
+                            switch (pCC->GetType())
+                            {
+                                case SwContentControlType::RICH_TEXT:
+                                    eKeyState = SwKeyState::InsTab;
+                                    break;
+                                default:
+                                    eKeyState = SwKeyState::GotoNextFieldMark;
+                            }
+                        }
+                    }
                     else
                     {
                         eKeyState = SwKeyState::InsTab;
@@ -2137,7 +2155,9 @@ KEYINPUT_CHECKTABLE_INSDEL:
                 break;
                 case KEY_TAB | KEY_SHIFT:
                 {
-                    if (rSh.IsFormProtected() || rSh.GetCurrentFieldmark()|| rSh.GetChar(false)==CH_TXT_ATR_FORMELEMENT)
+                    SwTextContentControl* pTextContentControl = rSh.CursorInsideContentControl();
+                    if ((rSh.IsFormProtected() && !pTextContentControl) ||
+                        rSh.GetCurrentFieldmark()|| rSh.GetChar(false)==CH_TXT_ATR_FORMELEMENT)
                     {
                         eKeyState = SwKeyState::GotoPrevFieldMark;
                     }
@@ -2176,6 +2196,10 @@ KEYINPUT_CHECKTABLE_INSDEL:
                             eNextKeyState = SwKeyState::PrevCell;
                         }
                     }
+                    else if (pTextContentControl)
+                    {
+                        eKeyState = SwKeyState::GotoPrevFieldMark;
+                    }
                     else
                     {
                         eKeyState = SwKeyState::End;
@@ -2610,18 +2634,13 @@ KEYINPUT_CHECKTABLE_INSDEL:
 
             case SwKeyState::GotoNextFieldMark:
                 {
-                    const sw::mark::IFieldmark* pFieldmark
-                        = rSh.GetFieldmarkAfter(/*bLoop=*/true);
-                    if(pFieldmark) rSh.GotoFieldmark(pFieldmark);
+                    rSh.GotoFormControl(/*bNext=*/true);
                 }
                 break;
 
             case SwKeyState::GotoPrevFieldMark:
                 {
-                    const sw::mark::IFieldmark* pFieldmark
-                        = rSh.GetFieldmarkBefore(/*bLoop=*/true);
-                    if( pFieldmark )
-                        rSh.GotoFieldmark(pFieldmark);
+                    rSh.GotoFormControl(/*bNext=*/false);
                 }
                 break;