After a fierce battle, Firefox support should be better, not perfect, but better.

src/cthulhu/scripts/toolkits/Gecko/script_utilities.py

@@ -61,13 +61,7 @@ class Utilities(web.Utilities):
         boundary = args.get('boundary')
 
         # Gecko fails to implement this boundary type.
-        if boundary == Atspi.TextBoundaryType.SENTENCE_START:
-            return True
-
-        if self.isContentEditableWithEmbeddedObjects(obj):
-            return boundary == Atspi.TextBoundaryType.WORD_START
-
-        return True
+        return boundary == Atspi.TextBoundaryType.SENTENCE_START
 
     def _treatAsLeafNode(self, obj):
         if AXUtilities.is_table_row(obj):

src/cthulhu/scripts/web/script_utilities.py

@@ -722,8 +722,15 @@ class Utilities(script_utilities.Utilities):
 
         nextobj, nextoffset = self.findNextCaretInOrder(obj, offset)
         if skipSpace:
-            while nextobj and AXText.get_character_at_offset(nextobj, nextoffset)[0].isspace():
+            seen = {(nextobj, nextoffset)}
+            while self.treatAsTextObject(nextobj) \
+                    and AXText.get_character_at_offset(nextobj, nextoffset)[0].isspace():
                 nextobj, nextoffset = self.findNextCaretInOrder(nextobj, nextoffset)
+                if (nextobj, nextoffset) in seen:
+                    msg = "WEB: Cycle detected in nextContext skipSpace. Breaking."
+                    debug.printMessage(debug.LEVEL_INFO, msg, True)
+                    break
+                seen.add((nextobj, nextoffset))
 
         return nextobj, nextoffset
 
@@ -733,8 +740,15 @@ class Utilities(script_utilities.Utilities):
 
         prevobj, prevoffset = self.findPreviousCaretInOrder(obj, offset)
         if skipSpace:
-            while prevobj and AXText.get_character_at_offset(prevobj, prevoffset)[0].isspace():
+            seen = {(prevobj, prevoffset)}
+            while self.treatAsTextObject(prevobj) \
+                    and AXText.get_character_at_offset(prevobj, prevoffset)[0].isspace():
                 prevobj, prevoffset = self.findPreviousCaretInOrder(prevobj, prevoffset)
+                if (prevobj, prevoffset) in seen:
+                    msg = "WEB: Cycle detected in previousContext skipSpace. Breaking."
+                    debug.printMessage(debug.LEVEL_INFO, msg, True)
+                    break
+                seen.add((prevobj, prevoffset))
 
         return prevobj, prevoffset
 

tests/test_web_input_regressions.py

@@ -10,6 +10,9 @@ os.environ.setdefault("GSETTINGS_BACKEND", "memory")
 
 gi.require_version("Gdk", "3.0")
 gi.require_version("Gtk", "3.0")
+gi.require_version("Atspi", "2.0")
+
+from gi.repository import Atspi
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
 
@@ -23,6 +26,7 @@ if soundGeneratorModule is not None and not hasattr(soundGeneratorModule, "Sound
 from cthulhu import messages
 from cthulhu.scripts.web import script as web_script
 from cthulhu.scripts.web import script_utilities as web_script_utilities
+from cthulhu.scripts.toolkits.Gecko import script_utilities as gecko_script_utilities
 
 
 class WebKeyGrabRegressionTests(unittest.TestCase):
@@ -716,6 +720,124 @@ class WebContentCacheRegressionTests(unittest.TestCase):
         utilities._cleanupContexts.assert_called_once_with()
 
 
+class WebSkipSpaceCaretNavigationRegressionTests(unittest.TestCase):
+    def _make_utilities(self):
+        utilities = web_script_utilities.Utilities.__new__(web_script_utilities.Utilities)
+        utilities.getCaretContext = mock.Mock(return_value=(None, -1))
+        utilities.treatAsTextObject = mock.Mock(return_value=True)
+        return utilities
+
+    def test_next_context_does_not_skip_named_whole_object_as_space(self):
+        utilities = self._make_utilities()
+        start = object()
+        wholeObject = object()
+        after = object()
+        utilities.treatAsTextObject.return_value = False
+        utilities.findNextCaretInOrder = mock.Mock(
+            side_effect=[(wholeObject, 0), (after, 0)]
+        )
+
+        with mock.patch.object(
+            web_script_utilities.AXText,
+            "get_character_at_offset",
+            return_value=(" ", 0, 1),
+        ) as getCharacter:
+            result = web_script_utilities.Utilities.nextContext(
+                utilities,
+                start,
+                0,
+                skipSpace=True,
+            )
+
+        self.assertEqual(result, (wholeObject, 0))
+        utilities.treatAsTextObject.assert_called_once_with(wholeObject)
+        getCharacter.assert_not_called()
+        utilities.findNextCaretInOrder.assert_called_once_with(start, 0)
+
+    def test_previous_context_does_not_skip_named_whole_object_as_space(self):
+        utilities = self._make_utilities()
+        start = object()
+        wholeObject = object()
+        before = object()
+        utilities.treatAsTextObject.return_value = False
+        utilities.findPreviousCaretInOrder = mock.Mock(
+            side_effect=[(wholeObject, 0), (before, 0)]
+        )
+
+        with mock.patch.object(
+            web_script_utilities.AXText,
+            "get_character_at_offset",
+            return_value=(" ", 0, 1),
+        ) as getCharacter:
+            result = web_script_utilities.Utilities.previousContext(
+                utilities,
+                start,
+                0,
+                skipSpace=True,
+            )
+
+        self.assertEqual(result, (wholeObject, 0))
+        utilities.treatAsTextObject.assert_called_once_with(wholeObject)
+        getCharacter.assert_not_called()
+        utilities.findPreviousCaretInOrder.assert_called_once_with(start, 0)
+
+    def test_next_context_breaks_skip_space_cycles(self):
+        utilities = self._make_utilities()
+        start = object()
+        first = object()
+        second = object()
+        utilities.findNextCaretInOrder = mock.Mock(
+            side_effect=[(first, 0), (second, 0), (first, 0)]
+        )
+
+        with mock.patch.object(
+            web_script_utilities.AXText,
+            "get_character_at_offset",
+            return_value=(" ", 0, 1),
+        ):
+            result = web_script_utilities.Utilities.nextContext(
+                utilities,
+                start,
+                0,
+                skipSpace=True,
+            )
+
+        self.assertEqual(result, (first, 0))
+        self.assertEqual(utilities.findNextCaretInOrder.call_args_list, [
+            mock.call(start, 0),
+            mock.call(first, 0),
+            mock.call(second, 0),
+        ])
+
+    def test_previous_context_breaks_skip_space_cycles(self):
+        utilities = self._make_utilities()
+        start = object()
+        first = object()
+        second = object()
+        utilities.findPreviousCaretInOrder = mock.Mock(
+            side_effect=[(first, 0), (second, 0), (first, 0)]
+        )
+
+        with mock.patch.object(
+            web_script_utilities.AXText,
+            "get_character_at_offset",
+            return_value=(" ", 0, 1),
+        ):
+            result = web_script_utilities.Utilities.previousContext(
+                utilities,
+                start,
+                0,
+                skipSpace=True,
+            )
+
+        self.assertEqual(result, (first, 0))
+        self.assertEqual(utilities.findPreviousCaretInOrder.call_args_list, [
+            mock.call(start, 0),
+            mock.call(first, 0),
+            mock.call(second, 0),
+        ])
+
+
 class WebSplitLinkLineNavigationRegressionTests(unittest.TestCase):
     def _make_utilities(self, link):
         utilities = web_script_utilities.Utilities.__new__(web_script_utilities.Utilities)
@@ -867,3 +989,32 @@ class WebSplitLinkLineNavigationRegressionTests(unittest.TestCase):
                 mock.call(paragraph, 96, layoutMode=False, useCache=False),
             ],
         )
+
+
+class WebGeckoBrokenTextRecoveryRegressionTests(unittest.TestCase):
+    def test_gecko_limits_broken_text_recovery_to_sentences(self):
+        utilities = gecko_script_utilities.Utilities.__new__(gecko_script_utilities.Utilities)
+        utilities.isContentEditableWithEmbeddedObjects = mock.Mock(return_value=True)
+        obj = object()
+
+        self.assertTrue(
+            gecko_script_utilities.Utilities._attemptBrokenTextRecovery(
+                utilities,
+                obj,
+                boundary=Atspi.TextBoundaryType.SENTENCE_START,
+            )
+        )
+        self.assertFalse(
+            gecko_script_utilities.Utilities._attemptBrokenTextRecovery(
+                utilities,
+                obj,
+                boundary=Atspi.TextBoundaryType.LINE_START,
+            )
+        )
+        self.assertFalse(
+            gecko_script_utilities.Utilities._attemptBrokenTextRecovery(
+                utilities,
+                obj,
+                boundary=Atspi.TextBoundaryType.WORD_START,
+            )
+        )