More refactor work.

src/cthulhu/ax_component.py

@@ -145,7 +145,7 @@ class AXComponent:
         return not(rect.width or rect.height)
 
     @staticmethod
-    def has_no_size_or_invalid_rect(obj: Atspi.Accessible) -> bool:
+    def has_no_size_or_invalid_rect(obj: Atspi.Accessible, clear_cache: bool = True) -> bool:
         """Returns True if the rect associated with obj is sizeless or invalid."""
 
         rect = AXComponent.get_rect(obj)
@@ -158,7 +158,8 @@ class AXComponent:
         if (rect.width < -1 or rect.height < -1):
             tokens = ["WARNING: ", obj, "has a broken rect:", rect]
             debug.print_tokens(debug.LEVEL_INFO, tokens, True)
-            AXObject.clear_cache(obj)
+            if clear_cache:
+                AXObject.clear_cache(obj)
             rect = AXComponent.get_rect(obj)
             if (rect.width < -1 or rect.height < -1):
                 msg = "AXComponent: Clearing cache did not fix the rect"

src/cthulhu/ax_utilities.py

@@ -109,13 +109,14 @@ class AXUtilities:
             AXTable.clear_cache_now(reason)
 
     @staticmethod
-    def can_be_active_window(window: Atspi.Accessible) -> bool:
+    def can_be_active_window(window: Atspi.Accessible, clear_cache: bool = True) -> bool:
         """Returns True if window can be the active window based on its state."""
 
         if window is None:
             return False
 
-        AXObject.clear_cache(window, False, "Checking if window can be the active window")
+        if clear_cache:
+            AXObject.clear_cache(window, False, "Checking if window can be the active window")
         app = AXUtilitiesApplication.get_application(window)
         tokens = ["AXUtilities:", window, "from", app]
 
@@ -808,11 +809,13 @@ class AXUtilities:
     @staticmethod
     def is_on_screen(
         obj: Atspi.Accessible,
-        bounding_box: Optional[Atspi.Rect] = None
+        bounding_box: Optional[Atspi.Rect] = None,
+        clear_cache: bool = True
     ) -> bool:
         """Returns true if obj should be treated as being on screen."""
 
-        AXObject.clear_cache(obj, False, "Updating to check if object is on screen.")
+        if clear_cache:
+            AXObject.clear_cache(obj, False, "Updating to check if object is on screen.")
 
         tokens = ["AXUtilities: Checking if", obj, "is showing and visible...."]
         debug.print_tokens(debug.LEVEL_INFO, tokens, True)
@@ -833,7 +836,7 @@ class AXUtilities:
         tokens = ["AXUtilities:", obj, "is not hidden. Checking size and rect..."]
         debug.print_tokens(debug.LEVEL_INFO, tokens, True)
 
-        if AXComponent.has_no_size_or_invalid_rect(obj):
+        if AXComponent.has_no_size_or_invalid_rect(obj, clear_cache=clear_cache):
             tokens = ["AXUtilities: Rect of", obj, "is unhelpful. Treating as on screen."]
             debug.print_tokens(debug.LEVEL_INFO, tokens, True)
             return True
@@ -972,6 +975,8 @@ class AXUtilities:
         debug.print_tokens(debug.LEVEL_INFO, tokens, True)
         return result
 
+# Dynamically expose helper methods for compatibility with older callers.
+# Keep side effects explicit in the underlying helpers (e.g., clear_cache flags).
 for method_name, method in inspect.getmembers(AXUtilitiesApplication, predicate=inspect.isfunction):
     setattr(AXUtilities, method_name, method)
 

src/cthulhu/event_manager.py

@@ -109,7 +109,7 @@ class EventManager:
             return False
 
         window = cthulhu_state.activeWindow
-        if not AXUtilities.can_be_active_window(window):
+        if not AXUtilities.can_be_active_window(window, clear_cache=True):
             window = AXUtilities.find_active_window()
             if window is not None:
                 tokens = ["EVENT MANAGER: Setting initial active window to", window]
@@ -237,40 +237,44 @@ class EventManager:
         tokens = ["EVENT MANAGER:", event.type, "from", app]
         debug.printTokens(debug.LEVEL_INFO, tokens, True)
 
+        def _log_ignore(reason, message):
+            msg = f"EVENT MANAGER: Ignoring ({reason}) - {message}"
+            debug.printMessage(debug.LEVEL_INFO, msg, True)
+
+        def _log_allow(reason, message):
+            msg = f"EVENT MANAGER: Not ignoring ({reason}) - {message}"
+            debug.printMessage(debug.LEVEL_INFO, msg, True)
+
+        def _ignore_with_reason(reason, message):
+            _log_ignore(reason, message)
+            return True
+
+        def _allow_with_reason(reason, message):
+            _log_allow(reason, message)
+            return False
+
         if self._eventsSuspended:
             tokens = ["EVENT MANAGER: Suspended events:", ', '.join(self._suspendableEvents)]
             debug.printTokens(debug.LEVEL_INFO, tokens, True)
 
         if not self._active:
-            msg = 'EVENT MANAGER: Ignoring because event manager is not active'
+            return _ignore_with_reason("inactive", "event manager is not active")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         if list(filter(event.type.startswith, self._ignoredEvents)):
-            msg = 'EVENT MANAGER: Ignoring because event type is ignored'
+            return _ignore_with_reason("type-ignored", "event type is ignored")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         if AXObject.get_name(app) == 'gnome-shell':
             if event.type.startswith('object:children-changed:remove'):
-                msg = 'EVENT MANAGER: Ignoring event based on type and app'
+                return _ignore_with_reason("gnome-shell", "children-changed:remove")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
         if event.type.startswith('window'):
-            msg = 'EVENT MANAGER: Not ignoring because event type is never ignored'
+            return _allow_with_reason("window-event", "event type is never ignored")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return False
 
         if event.type.startswith('mouse:button'):
-            msg = 'EVENT MANAGER: Not ignoring because event type is never ignored'
+            return _allow_with_reason("mouse-event", "event type is never ignored")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return False
 
         if self._isDuplicateEvent(event):
-            msg = 'EVENT MANAGER: Ignoring duplicate event'
+            return _ignore_with_reason("duplicate", "duplicate event")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         # Thunderbird spams us with these when a message list thread is expanded or collapsed.
         if event.type.endswith('system') \
@@ -278,40 +282,29 @@ class EventManager:
             if AXUtilities.is_table_related(event.source) \
               or AXUtilities.is_tree_related(event.source) \
               or AXUtilities.is_section(event.source):
-                msg = 'EVENT MANAGER: Ignoring system event based on role'
+                return _ignore_with_reason("thunderbird-system", "system event based on role")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
         if self._inDeluge() and self._ignoreDuringDeluge(event):
-            msg = 'EVENT MANAGER: Ignoring event type due to deluge'
+            return _ignore_with_reason("deluge", "event type during deluge")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         script = cthulhu_state.activeScript
         if event.type.startswith('object:children-changed') \
            or event.type.startswith('object:state-changed:sensitive'):
             if not script:
-                msg = 'EVENT MANAGER: Ignoring because there is no active script'
+                return _ignore_with_reason("no-active-script", "no active script")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
             if script.app != app:
                 # Allow Steam notifications from inactive apps.
                 if self._isSteamApp(app) and self._isSteamNotificationEvent(event):
-                    msg = 'EVENT MANAGER: Allowing Steam notification from inactive app'
+                    _log_allow("steam-notification", "inactive app notification")
-                    debug.printMessage(debug.LEVEL_INFO, msg, True)
                 else:
-                    msg = 'EVENT MANAGER: Ignoring because event is not from active app'
+                    return _ignore_with_reason("inactive-app", "event not from active app")
-                    debug.printMessage(debug.LEVEL_INFO, msg, True)
-                    return True
 
         if event.type.startswith('object:text-changed') \
            and self.EMBEDDED_OBJECT_CHARACTER in event.any_data \
            and not event.any_data.replace(self.EMBEDDED_OBJECT_CHARACTER, ""):
             # We should also get children-changed events telling us the same thing.
             # Getting a bunch of both can result in a flood that grinds us to a halt.
-            msg = 'EVENT MANAGER: Ignoring because changed text is only embedded objects'
+            return _ignore_with_reason("embedded-only", "changed text only embedded objects")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         # TODO - JD: For now we won't ask for the name. Simply asking for the name should
         # not break anything, and should be a reliable way to quickly identify defunct
@@ -321,14 +314,10 @@ class EventManager:
         #name = Atspi.Accessible.get_name(event.source)
 
         if AXUtilities.has_no_state(event.source):
-            msg = 'EVENT MANAGER: Ignoring event due to empty state set'
+            return _ignore_with_reason("empty-state", "empty state set")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         if AXUtilities.is_defunct(event.source):
-            msg = 'EVENT MANAGER: Ignoring event from defunct source'
+            return _ignore_with_reason("defunct-source", "defunct source")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
 
         role = AXObject.get_role(event.source)
         if event.type.startswith('object:property-change:accessible-name'):
@@ -344,28 +333,20 @@ class EventManager:
                         Atspi.Role.IMAGE,      # Thunderbird spam
                         Atspi.Role.MENU,
                         Atspi.Role.MENU_ITEM]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role'
+                return _ignore_with_reason("name-change-role", "role filtered")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
             # TeamTalk5 is notoriously spammy here, and name change events on widgets are
             # typically only presented if they are focused.
             if not AXUtilities.is_focused(event.source) \
                and role in [Atspi.Role.PUSH_BUTTON,
                             Atspi.Role.CHECK_BOX,
                             Atspi.Role.RADIO_BUTTON]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role and state'
+                return _ignore_with_reason("name-change-unfocused", "role and state")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
         elif event.type.startswith('object:property-change:accessible-value'):
             if role == Atspi.Role.SPLIT_PANE and not AXUtilities.is_focused(event.source):
-                msg = 'EVENT MANAGER: Ignoring event type due to role and state'
+                return _ignore_with_reason("value-change-unfocused", "role and state")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
         elif event.type.startswith('object:text-changed:insert') and event.detail2 > 1000 \
              and role in [Atspi.Role.TEXT, Atspi.Role.STATIC]:
-            msg = 'EVENT MANAGER: Ignoring because inserted text has more than 1000 chars'
+            return _ignore_with_reason("text-insert-large", "inserted text > 1000 chars")
-            debug.printMessage(debug.LEVEL_INFO, msg, True)
-            return True
         elif event.type.startswith('object:state-changed:sensitive'):
             if role in [Atspi.Role.MENU_ITEM,
                         Atspi.Role.MENU,
@@ -373,14 +354,10 @@ class EventManager:
                         Atspi.Role.PANEL,
                         Atspi.Role.CHECK_MENU_ITEM,
                         Atspi.Role.RADIO_MENU_ITEM]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role'
+                return _ignore_with_reason("sensitive-role", "role filtered")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
         elif event.type.startswith('object:state-changed:selected'):
             if not event.detail1 and role in [Atspi.Role.PUSH_BUTTON]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role and detail1'
+                return _ignore_with_reason("selected-button-false", "role and detail1")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
         elif event.type.startswith('object:state-changed:showing'):
             if role not in [Atspi.Role.ALERT,
                             Atspi.Role.ANIMATION,
@@ -390,39 +367,27 @@ class EventManager:
                             Atspi.Role.DIALOG,
                             Atspi.Role.STATUS_BAR,
                             Atspi.Role.TOOL_TIP]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role'
+                return _ignore_with_reason("showing-role", "role filtered")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
         elif event.type.startswith('object:text-caret-moved'):
             if role in [Atspi.Role.LABEL] and not AXUtilities.is_focused(event.source):
-                msg = 'EVENT MANAGER: Ignoring event type due to role and state'
+                return _ignore_with_reason("caret-unfocused-label", "role and state")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
         elif event.type.startswith('object:selection-changed'):
             if event.source in self._parentsOfDefunctDescendants:
-                msg = 'EVENT MANAGER: Ignoring event from parent of defunct descendants'
+                return _ignore_with_reason("defunct-descendant-parent", "parent of defunct descendants")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
             if AXObject.is_dead(event.source):
-                msg = 'EVENT MANAGER: Ignoring event from dead source'
+                return _ignore_with_reason("dead-source", "dead source")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
         if event.type.startswith('object:children-changed') \
            or event.type.startswith('object:active-descendant-changed'):
             if role in [Atspi.Role.MENU,
                         Atspi.Role.LAYERED_PANE,
                         Atspi.Role.MENU_ITEM]:
-                msg = 'EVENT MANAGER: Ignoring event type due to role'
+                return _ignore_with_reason("children-role", "role filtered")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
             if event.any_data is None:
-                msg = 'EVENT_MANAGER: Ignoring due to lack of event.any_data'
+                return _ignore_with_reason("missing-any-data", "no event.any_data")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
             if event.type.endswith('remove'):
                 if event.any_data == cthulhu_state.locusOfFocus:
                     msg = 'EVENT MANAGER: Locus of focus is being destroyed'
@@ -439,8 +404,7 @@ class EventManager:
 
             defunct = AXObject.is_dead(event.any_data) or AXUtilities.is_defunct(event.any_data)
             if defunct:
-                msg = 'EVENT MANAGER: Ignoring event for potentially-defunct child/descendant'
+                _log_ignore("defunct-child", "potentially defunct child/descendant")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
                 if AXUtilities.manages_descendants(event.source) \
                    and event.source not in self._parentsOfDefunctDescendants:
                     self._parentsOfDefunctDescendants.append(event.source)
@@ -455,21 +419,15 @@ class EventManager:
             # reason for ignoring it here rather than quickly processing it is the
             # potential for event floods like we're seeing from matrix.org.
             if AXUtilities.is_image(event.any_data):
-                msg = 'EVENT MANAGER: Ignoring event type due to role'
+                return _ignore_with_reason("child-image", "role filtered")
-                debug.printMessage(debug.LEVEL_INFO, msg, True)
-                return True
 
             # In normal apps we would have caught this from the parent role.
             # But gnome-shell has panel parents adding/removing menu items.
             if event.type.startswith('object:children-changed'):
                 if AXUtilities.is_menu_item(event.any_data):
-                    msg = 'EVENT MANAGER: Ignoring event type due to child role'
+                    return _ignore_with_reason("child-menu-item", "child role filtered")
-                    debug.printMessage(debug.LEVEL_INFO, msg, True)
-                    return True
 
-        msg = 'EVENT MANAGER: Not ignoring due to lack of cause'
+        return _allow_with_reason("no-cause", "no ignore condition met")
-        debug.printMessage(debug.LEVEL_INFO, msg, True)
-        return False
 
     def _addToQueue(self, event, asyncMode):
         debugging = debug.debugEventQueue

src/cthulhu/flat_review.py

@@ -111,24 +111,29 @@ class Word:
         if attr != "chars":
             return super().__getattribute__(attr)
 
+        chars = []
+        for i, char in enumerate(self.string):
+            start = i + self.startOffset
+            extents = self._getCharExtents(start)
+            chars.append(Char(self, i, start, char, *extents))
+
+        return chars
+
+    def _getCharExtents(self, start):
         # TODO - JD: For now, don't fake character and word extents.
         # The main goal is to improve reviewability.
         extents = self.x, self.y, self.width, self.height
 
-        chars = []
+        if AXObject.supports_text(self.zone.accessible):
-        for i, char in enumerate(self.string):
+            try:
-            start = i + self.startOffset
+                rect = Atspi.Text.get_range_extents(
-            if AXObject.supports_text(self.zone.accessible):
+                    self.zone.accessible, start, start + 1, Atspi.CoordType.SCREEN)
-                try:
+                extents = rect.x, rect.y, rect.width, rect.height
-                    rect = Atspi.Text.get_range_extents(
+            except Exception as error:
-                        self.zone.accessible, start, start + 1, Atspi.CoordType.SCREEN)
+                tokens = ["FLAT REVIEW: Exception in getRangeExtents:", error]
-                    extents = rect.x, rect.y, rect.width, rect.height
+                debug.printTokens(debug.LEVEL_INFO, tokens, True)
-                except Exception as error:
-                    tokens = ["FLAT REVIEW: Exception in getRangeExtents:", error]
-                    debug.printTokens(debug.LEVEL_INFO, tokens, True)
-            chars.append(Char(self, i, start, char, *extents))
 
-        return chars
+        return extents
 
     def getRelativeOffset(self, offset):
         """Returns the char offset with respect to this word or -1."""
@@ -143,6 +148,16 @@ class Zone:
     """Represents text that is a portion of a single horizontal line."""
 
     WORDS_RE = re.compile(r"(\S+\s*)", re.UNICODE)
+    TEXT_ROLES = (
+        Atspi.Role.LABEL,
+        Atspi.Role.MENU,
+        Atspi.Role.MENU_ITEM,
+        Atspi.Role.CHECK_MENU_ITEM,
+        Atspi.Role.RADIO_MENU_ITEM,
+        Atspi.Role.PAGE_TAB,
+        Atspi.Role.PUSH_BUTTON,
+        Atspi.Role.TABLE_CELL,
+    )
 
     def __init__(self, accessible, string, x, y, width, height, role=None):
         """Creates a new Zone.
@@ -180,10 +195,7 @@ class Zone:
         if not self._shouldFakeText():
             return self._words
 
-        # TODO - JD: For now, don't fake character and word extents.
+        extents = self._getFakeTextExtents()
-        # The main goal is to improve reviewability.
-        extents = self.x, self.y, self.width, self.height
-
         words = []
         for i, word in enumerate(re.finditer(self.WORDS_RE, self._string)):
             words.append(Word(self, i, word.start(), word.group(), *extents))
@@ -194,20 +206,16 @@ class Zone:
     def _shouldFakeText(self):
         """Returns True if we should try to fake the text interface"""
 
-        textRoles = [Atspi.Role.LABEL,
+        if self.role in self.TEXT_ROLES:
-                     Atspi.Role.MENU,
-                     Atspi.Role.MENU_ITEM,
-                     Atspi.Role.CHECK_MENU_ITEM,
-                     Atspi.Role.RADIO_MENU_ITEM,
-                     Atspi.Role.PAGE_TAB,
-                     Atspi.Role.PUSH_BUTTON,
-                     Atspi.Role.TABLE_CELL]
-
-        if self.role in textRoles:
             return True
 
         return False
 
+    def _getFakeTextExtents(self):
+        # TODO - JD: For now, don't fake character and word extents.
+        # The main goal is to improve reviewability.
+        return self.x, self.y, self.width, self.height
+
     def _extentsAreOnSameLine(self, zone, pixelDelta=5):
         """Returns True if this Zone is physically on the same line as zone."""
 
@@ -291,7 +299,7 @@ class TextZone(Zone):
         string = AXText.get_substring(self.accessible, self.startOffset, self.endOffset)
         words = []
         for i, word in enumerate(re.finditer(self.WORDS_RE, string)):
-            start, end = map(lambda x: x + self.startOffset, word.span())
+            start, end = (pos + self.startOffset for pos in word.span())
             try:
                 rect = Atspi.Text.get_range_extents(self.accessible, start, end, Atspi.CoordType.SCREEN)
                 extents = rect.x, rect.y, rect.width, rect.height
@@ -338,6 +346,9 @@ class StateZone(Zone):
         else:
             generator = cthulhu_state.activeScript.brailleGenerator
 
+        return self._getStateString(generator)
+
+    def _getStateString(self, generator):
         result = generator.getStateIndicator(self.accessible, role=self.role)
         if result:
             return result[0]
@@ -362,21 +373,33 @@ class ValueZone(Zone):
         else:
             generator = cthulhu_state.activeScript.brailleGenerator
 
-        result = ""
+        return self._getValueString(generator)
 
+    def _getValueString(self, generator):
         # TODO - JD: This cobbling together beats what we had, but the
         # generators should also be doing the assembly.
         rolename = generator.getLocalizedRoleName(self.accessible)
         value = generator.getValue(self.accessible)
         if rolename and value:
-            result = f"{rolename} {value[0]}"
+            return f"{rolename} {value[0]}"
 
-        return result
+        return ""
 
 
 class Line:
     """A Line is a single line across a window and is composed of Zones."""
 
+    TEXT_BRAILLE_ROLES = (
+        Atspi.Role.TEXT,
+        Atspi.Role.PASSWORD_TEXT,
+        Atspi.Role.TERMINAL,
+    )
+    TEXT_BRAILLE_FALLBACK_ROLES = (
+        Atspi.Role.PARAGRAPH,
+        Atspi.Role.HEADING,
+        Atspi.Role.LINK,
+    )
+
     def __init__(self,
                  index,
                  zones):
@@ -395,16 +418,16 @@ class Line:
             return " ".join([zone.string for zone in self.zones])
 
         if attr == "x":
-            return min([zone.x for zone in self.zones])
+            return min(zone.x for zone in self.zones)
 
         if attr == "y":
-            return min([zone.y for zone in self.zones])
+            return min(zone.y for zone in self.zones)
 
         if attr == "width":
-            return sum([zone.width for zone in self.zones])
+            return sum(zone.width for zone in self.zones)
 
         if attr == "height":
-            return max([zone.height for zone in self.zones])
+            return max(zone.height for zone in self.zones)
 
         return super().__getattribute__(attr)
 
@@ -420,19 +443,13 @@ class Line:
                 # The 'isinstance(zone, TextZone)' test is a sanity check
                 # to handle problems with Java text. See Bug 435553.
                 if isinstance(zone, TextZone) and \
-                   ((AXObject.get_role(zone.accessible) in \
+                   ((AXObject.get_role(zone.accessible) in self.TEXT_BRAILLE_ROLES) or \
-                         (Atspi.Role.TEXT,  
-                          Atspi.Role.PASSWORD_TEXT,
-                          Atspi.Role.TERMINAL)) or \
                     # [[[TODO: Eitan - HACK: 
                     # This is just to get FF3 cursor key routing support.
                     # We really should not be determining all this stuff here,
                     # it should be in the scripts. 
                     # Same applies to roles above.]]]
-                    (AXObject.get_role(zone.accessible) in \
+                    (AXObject.get_role(zone.accessible) in self.TEXT_BRAILLE_FALLBACK_ROLES)):
-                         (Atspi.Role.PARAGRAPH,
-                          Atspi.Role.HEADING,
-                          Atspi.Role.LINK))):
                     region = braille.ReviewText(zone.accessible,
                                                 zone.string,
                                                 zone.startOffset,
@@ -485,6 +502,17 @@ class Context:
     WRAP_TOP_BOTTOM = 1 << 1
     WRAP_ALL        = (WRAP_LINE | WRAP_TOP_BOTTOM)
 
+    CONTAINER_ROLES = (Atspi.Role.MENU,)
+    VALUE_ZONE_ROLES = (Atspi.Role.SCROLL_BAR, Atspi.Role.SLIDER, Atspi.Role.PROGRESS_BAR)
+    REDUNDANT_NAME_ROLES = (Atspi.Role.TABLE_ROW,)
+    USELESS_NAME_ROLES = (Atspi.Role.TABLE_CELL, Atspi.Role.LABEL)
+    STATE_ZONE_ROLES = (
+        Atspi.Role.CHECK_BOX,
+        Atspi.Role.CHECK_MENU_ITEM,
+        Atspi.Role.RADIO_BUTTON,
+        Atspi.Role.RADIO_MENU_ITEM,
+    )
+
     def __init__(self, script, root=None):
         """Create a new Context for script."""
 
@@ -518,10 +546,8 @@ class Context:
             tokens = ["ERROR: Exception getting extents of", self.topLevel]
             debug.printTokens(debug.LEVEL_INFO, tokens, True)
 
-        containerRoles = [Atspi.Role.MENU]
-
         def isContainer(x):
-            return AXObject.get_role(x) in containerRoles
+            return AXObject.get_role(x) in self.CONTAINER_ROLES
 
         container = AXObject.find_ancestor(self.focusObj, isContainer)
         if not container and isContainer(self.focusObj):
@@ -567,8 +593,9 @@ class Context:
 
         cliprect = self._ensureRect(cliprect)
         zones = []
-        substrings = [(*m.span(), m.group(0))  for m in re.finditer(r"[^\ufffc]+", string)]
+        substrings = [(*m.span(), m.group(0))  for m in EMBEDDED_OBJECT_RE.finditer(string)]
-        substrings = list(map(lambda x: (x[0] + startOffset, x[1] + startOffset, x[2]), substrings))
+        substrings = [(start + startOffset, end + startOffset, text)
+                      for (start, end, text) in substrings]
         for (start, end, substring) in substrings:
             try:
                 rect = Atspi.Text.get_range_extents(accessible, start, end, Atspi.CoordType.SCREEN)
@@ -581,23 +608,6 @@ class Context:
 
         return zones
 
-    def _getLines(self, accessible, startOffset, endOffset):
-        # TODO - JD: Move this into the script utilities so we can better handle
-        # app and toolkit quirks and also reuse this (e.g. for SayAll).
-        if not AXObject.supports_text(accessible):
-            return []
-
-        lines = []
-        offset = startOffset
-        maxOffset = min(endOffset, AXText.get_character_count(accessible))
-        while offset < maxOffset:
-            line, start, end = AXText.get_line_at_offset(accessible, offset)
-            if line and (line, start, end) not in lines:
-                lines.append((line, start, end))
-            offset = max(end, offset + 1)
-
-        return lines
-
     def getZonesFromText(self, accessible, cliprect):
         """Gets a list of Zones from an object that implements the
         AccessibleText specialization.
@@ -613,15 +623,9 @@ class Context:
         if not self.script.utilities.hasPresentableText(accessible):
             return []
 
-        zones = []
+        zones = self._getSingleLineEditableZones(accessible)
-
+        if zones:
-        # TODO - JD: This is here temporarily whilst I sort out the rest
+            return zones
-        # of the text-related mess.
-        if AXObject.supports_editable_text(accessible) \
-           and AXUtilities.is_single_line(accessible):
-            rect = AXComponent.get_rect(accessible)
-            return [TextZone(accessible, 0, AXText.get_substring(accessible, 0, -1),
-                             rect.x, rect.y, rect.width, rect.height)]
 
         upperMax = lowerMax = AXText.get_character_count(accessible)
         upperMid = lowerMid = int(upperMax / 2)
@@ -657,7 +661,7 @@ class Context:
         msg = "FLAT REVIEW: Getting lines for %s offsets %i-%i" % (accessible, upperMin, lowerMax)
         debug.printMessage(debug.LEVEL_INFO, msg, True)
 
-        lines = self._getLines(accessible, upperMin, lowerMax)
+        lines = self.script.utilities.getLinesForRange(accessible, upperMin, lowerMax)
         tokens = ["FLAT REVIEW:", len(lines), "lines found for", accessible]
         debug.printTokens(debug.LEVEL_INFO, tokens, True)
 
@@ -666,6 +670,17 @@ class Context:
 
         return zones
 
+    def _getSingleLineEditableZones(self, accessible):
+        # TODO - JD: This is here temporarily whilst I sort out the rest
+        # of the text-related mess.
+        if not (AXObject.supports_editable_text(accessible)
+                and AXUtilities.is_single_line(accessible)):
+            return []
+
+        rect = AXComponent.get_rect(accessible)
+        return [TextZone(accessible, 0, AXText.get_substring(accessible, 0, -1),
+                         rect.x, rect.y, rect.width, rect.height)]
+
     def _insertStateZone(self, zones, accessible, extents):
         """If the accessible presents non-textual state, such as a
         checkbox or radio button, insert a StateZone representing
@@ -687,10 +702,7 @@ class Context:
            and self.script.utilities.hasMeaningfulToggleAction(accessible):
             role = Atspi.Role.CHECK_BOX
 
-        if role not in [Atspi.Role.CHECK_BOX,
+        if role not in self.STATE_ZONE_ROLES:
-                        Atspi.Role.CHECK_MENU_ITEM,
-                        Atspi.Role.RADIO_BUTTON,
-                        Atspi.Role.RADIO_MENU_ITEM]:
             return
 
         zone = None
@@ -733,18 +745,14 @@ class Context:
             return []
 
         zones = self.getZonesFromText(accessible, cliprect)
-        if not zones and role in [Atspi.Role.SCROLL_BAR,
+        if not zones and role in self.VALUE_ZONE_ROLES:
-                                  Atspi.Role.SLIDER,
-                                  Atspi.Role.PROGRESS_BAR]:
             zones.append(ValueZone(accessible, *extents))
         elif not zones:
             string = ""
-            redundant = [Atspi.Role.TABLE_ROW]
+            if role not in self.REDUNDANT_NAME_ROLES:
-            if role not in redundant:
                 string = self.script.speechGenerator.getName(accessible, inFlatReview=True)
 
-            useless = [Atspi.Role.TABLE_CELL, Atspi.Role.LABEL]
+            if not string and role not in self.USELESS_NAME_ROLES:
-            if not string and role not in useless:
                 string = self.script.speechGenerator.getRoleName(accessible)
             if string:
                 zones.append(Zone(accessible, string, *extents))
@@ -1467,3 +1475,4 @@ class Context:
             raise Exception("Invalid type: %d" % flatReviewType)
 
         return moved
+EMBEDDED_OBJECT_RE = re.compile(r"[^\ufffc]+")

src/cthulhu/input_event_manager.py

@@ -332,7 +332,7 @@ class InputEventManager:
         manager = focus_manager.get_manager()
         if pressed:
             window = manager.get_active_window()
-            if not AXUtilities.can_be_active_window(window):
+            if not AXUtilities.can_be_active_window(window, clear_cache=True):
                 new_window = AXUtilities.find_active_window()
                 if new_window is not None:
                     window = new_window

src/cthulhu/script_utilities.py

@@ -101,6 +101,22 @@ class Utilities:
     SUBSCRIPT_DIGITS = \
         ['\u2080', '\u2081', '\u2082', '\u2083', '\u2084',
          '\u2085', '\u2086', '\u2087', '\u2088', '\u2089']
+    MENU_ROLES_IN_OPEN_MENU = {
+        Atspi.Role.MENU,
+        Atspi.Role.MENU_ITEM,
+        Atspi.Role.CHECK_MENU_ITEM,
+        Atspi.Role.RADIO_MENU_ITEM,
+        Atspi.Role.SEPARATOR,
+    }
+    ZOMBIE_TOP_LEVEL_ROLES = {
+        Atspi.Role.APPLICATION,
+        Atspi.Role.ALERT,
+        Atspi.Role.DIALOG,
+        Atspi.Role.LABEL, # For Unity Panel Service bug
+        Atspi.Role.PAGE, # For Evince bug
+        Atspi.Role.WINDOW,
+        Atspi.Role.FRAME,
+    }
 
     flags = re.UNICODE
     WORDS_RE = re.compile(r"(\W+)", flags)
@@ -3687,6 +3703,21 @@ class Utilities:
             debug.printException(debug.LEVEL_WARNING)
             return ""
 
+    def getLinesForRange(self, obj, startOffset, endOffset):
+        if not AXObject.supports_text(obj):
+            return []
+
+        lines = []
+        offset = startOffset
+        maxOffset = min(endOffset, AXText.get_character_count(obj))
+        while offset < maxOffset:
+            line, start, end = AXText.get_line_at_offset(obj, offset)
+            if line and (line, start, end) not in lines:
+                lines.append((line, start, end))
+            offset = max(end, offset + 1)
+
+        return lines
+
     def getLineContentsAtOffset(self, obj, offset, layoutMode=True, useCache=True):
         return []
 
@@ -4573,30 +4604,23 @@ class Utilities:
         # seems to be present in multiple toolkits, so it's either being
         # inherited (e.g. from Gtk in Firefox Chrome, LO, Eclipse) or it
         # may be an AT-SPI2 bug. For now, handling it here.
-        menuRoles = [Atspi.Role.MENU,
+        if self._is_open_menu_bar_menu_role(obj):
-                     Atspi.Role.MENU_ITEM,
-                     Atspi.Role.CHECK_MENU_ITEM,
-                     Atspi.Role.RADIO_MENU_ITEM,
-                     Atspi.Role.SEPARATOR]
-        if AXObject.get_role(obj) in menuRoles and self.isInOpenMenuBarMenu(obj):
             tokens = ["HACK: Treating", obj, "as showing and visible"]
             debug.printTokens(debug.LEVEL_INFO, tokens, True)
             return True
 
         return False
 
+    def _is_open_menu_bar_menu_role(self, obj):
+        if AXObject.get_role(obj) not in self.MENU_ROLES_IN_OPEN_MENU:
+            return False
+        return self.isInOpenMenuBarMenu(obj)
+
     def isZombie(self, obj):
         index = AXObject.get_index_in_parent(obj)
-        topLevelRoles = [Atspi.Role.APPLICATION,
-                         Atspi.Role.ALERT,
-                         Atspi.Role.DIALOG,
-                         Atspi.Role.LABEL, # For Unity Panel Service bug
-                         Atspi.Role.PAGE, # For Evince bug
-                         Atspi.Role.WINDOW,
-                         Atspi.Role.FRAME]
         role = AXObject.get_role(obj)
         tokens = ["SCRIPT UTILITIES: ", obj, "is zombie:"]
-        if index == -1 and role not in topLevelRoles:
+        if index == -1 and role not in self.ZOMBIE_TOP_LEVEL_ROLES:
             tokens.append("index is -1")
             debug.printTokens(debug.LEVEL_INFO, tokens, True)
             return True