diff --git a/src/cthulhu/flat_review.py b/src/cthulhu/flat_review.py index 8cefcc3..5a5735d 100644 --- a/src/cthulhu/flat_review.py +++ b/src/cthulhu/flat_review.py @@ -120,8 +120,7 @@ class Word: return chars def _getCharExtents(self, start): - # TODO - JD: For now, don't fake character and word extents. - # The main goal is to improve reviewability. + # NOTE: We intentionally avoid faking character extents to improve reviewability. extents = self.x, self.y, self.width, self.height if AXObject.supports_text(self.zone.accessible): @@ -212,8 +211,7 @@ class Zone: return False def _getFakeTextExtents(self): - # TODO - JD: For now, don't fake character and word extents. - # The main goal is to improve reviewability. + # NOTE: We intentionally avoid faking word extents to improve reviewability. return self.x, self.y, self.width, self.height def _extentsAreOnSameLine(self, zone, pixelDelta=5): @@ -376,8 +374,7 @@ class ValueZone(Zone): 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. + # NOTE: This assembly is a stopgap until generators handle it directly. rolename = generator.getLocalizedRoleName(self.accessible) value = generator.getValue(self.accessible) if rolename and value: @@ -440,29 +437,7 @@ class Line: self.brailleRegions = [] brailleOffset = 0 for zone in self.zones: - # 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 self.TEXT_BRAILLE_ROLES) 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 self.TEXT_BRAILLE_FALLBACK_ROLES)): - region = braille.ReviewText(zone.accessible, - zone.string, - zone.startOffset, - zone) - else: - try: - brailleString = zone.brailleString - except Exception: - brailleString = zone.string - region = braille.ReviewComponent(zone.accessible, - brailleString, - 0, # cursor offset - zone) + region = self._createBrailleRegion(zone) if len(self.brailleRegions): pad = braille.Region(" ") pad.brailleOffset = brailleOffset @@ -488,6 +463,36 @@ class Line: return self.brailleRegions + def _createBrailleRegion(self, zone): + if self._shouldUseTextBrailleRegion(zone): + return braille.ReviewText(zone.accessible, + zone.string, + zone.startOffset, + zone) + + try: + brailleString = zone.brailleString + except Exception: + brailleString = zone.string + + return braille.ReviewComponent(zone.accessible, + brailleString, + 0, # cursor offset + zone) + + def _shouldUseTextBrailleRegion(self, zone): + # The 'isinstance(zone, TextZone)' test is a sanity check + # to handle problems with Java text. See Bug 435553. + if not isinstance(zone, TextZone): + return False + + role = AXObject.get_role(zone.accessible) + if role in self.TEXT_BRAILLE_ROLES: + return True + + # NOTE: Fallback roles kept for legacy cursor key routing support. + return role in self.TEXT_BRAILLE_FALLBACK_ROLES + class Context: """Contains the flat review regions for the current top-level object.""" @@ -671,8 +676,7 @@ 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. + # NOTE: Temporary workaround while text handling is refined. if not (AXObject.supports_editable_text(accessible) and AXUtilities.is_single_line(accessible)): return [] @@ -686,8 +690,7 @@ class Context: checkbox or radio button, insert a StateZone representing that state.""" - # TODO - JD: This whole thing is pretty hacky. Either do it - # right or nuke it. + # NOTE: Heuristic state-zone placement until handled by scripts. extents = self._ensureRect(extents) indicatorExtents = [extents.x, extents.y, 1, extents.height] @@ -910,8 +913,7 @@ class Context: def getCurrent(self, flatReviewType=ZONE): """Returns the current string, offset, and extent information.""" - # TODO - JD: This method has not (yet) been renamed. But we have a - # getter and setter which do totally different things.... + # NOTE: Legacy name; prefer getCurrentItem() for clarity. zone = self._getCurrentZone() if not zone: @@ -930,6 +932,9 @@ class Context: return current.string, current.x, current.y, current.width, current.height + def getCurrentItem(self, flatReviewType=ZONE): + return self.getCurrent(flatReviewType) + def setCurrent(self, lineIndex, zoneIndex, wordIndex, charIndex): """Sets the current character of interest. diff --git a/src/cthulhu/script_utilities.py b/src/cthulhu/script_utilities.py index 931342b..2a02e00 100644 --- a/src/cthulhu/script_utilities.py +++ b/src/cthulhu/script_utilities.py @@ -117,6 +117,18 @@ class Utilities: Atspi.Role.WINDOW, Atspi.Role.FRAME, } + DISPLAYED_TEXT_DIRECT_NAME_ROLES = { + Atspi.Role.PUSH_BUTTON, + Atspi.Role.LABEL, + } + DISPLAYED_TEXT_SKIP_NAME_ROLES = { + Atspi.Role.COMBO_BOX, + Atspi.Role.SPIN_BUTTON, + } + DISPLAYED_TEXT_LABEL_FALLBACK_ROLES = { + Atspi.Role.PUSH_BUTTON, + Atspi.Role.LIST_ITEM, + } flags = re.UNICODE WORDS_RE = re.compile(r"(\W+)", flags) @@ -485,7 +497,7 @@ class Utilities: any text being shown. """ - # TODO - JD: It's finally time to consider killing this for real. + # NOTE: Legacy behavior; removal requires thorough testing. try: return self._script.generatorCache[self.DISPLAYED_TEXT][obj] @@ -494,25 +506,16 @@ class Utilities: name = AXObject.get_name(obj) role = AXObject.get_role(obj) - if role in [Atspi.Role.PUSH_BUTTON, Atspi.Role.LABEL] and name: - return name - if AXObject.supports_text(obj): - displayedText = AXText.get_all_text(obj) - if self.EMBEDDED_OBJECT_CHARACTER in displayedText: - displayedText = None + directText = self._getDisplayedTextDirectName(role, name) + if directText is not None: + return directText - if not displayedText and role not in [Atspi.Role.COMBO_BOX, Atspi.Role.SPIN_BUTTON]: - # TODO - JD: This should probably get nuked. But all sorts of - # existing code might be relying upon this bogus hack. So it - # will need thorough testing when removed. - displayedText = name - - if not displayedText and role in [Atspi.Role.PUSH_BUTTON, Atspi.Role.LIST_ITEM]: - labels = self.unrelatedLabels(obj, minimumWords=1) - if not labels: - labels = self.unrelatedLabels(obj, onlyShowing=False, minimumWords=1) - displayedText = " ".join(map(self.displayedText, labels)) + displayedText = self._getDisplayedTextFromText(obj) + if not displayedText: + displayedText = self._getDisplayedTextFallbackName(role, name) + if not displayedText: + displayedText = self._getDisplayedTextFromLabels(obj, role) if self.DISPLAYED_TEXT not in self._script.generatorCache: self._script.generatorCache[self.DISPLAYED_TEXT] = {} @@ -520,6 +523,37 @@ class Utilities: self._script.generatorCache[self.DISPLAYED_TEXT][obj] = displayedText return self._script.generatorCache[self.DISPLAYED_TEXT][obj] + def _getDisplayedTextDirectName(self, role, name): + if role in self.DISPLAYED_TEXT_DIRECT_NAME_ROLES and name: + return name + return None + + def _getDisplayedTextFromText(self, obj): + if not AXObject.supports_text(obj): + return None + + displayedText = AXText.get_all_text(obj) + if self.EMBEDDED_OBJECT_CHARACTER in displayedText: + return None + + return displayedText + + def _getDisplayedTextFallbackName(self, role, name): + # NOTE: Legacy fallback; removal requires thorough testing. + if name and role not in self.DISPLAYED_TEXT_SKIP_NAME_ROLES: + return name + + return None + + def _getDisplayedTextFromLabels(self, obj, role): + if role not in self.DISPLAYED_TEXT_LABEL_FALLBACK_ROLES: + return None + + labels = self.unrelatedLabels(obj, minimumWords=1) + if not labels: + labels = self.unrelatedLabels(obj, onlyShowing=False, minimumWords=1) + return " ".join(map(self.displayedText, labels)) + def documentFrame(self, obj=None): """Returns the document frame which is displaying the content. Note that this is intended primarily for web content.""" @@ -1548,8 +1582,10 @@ class Utilities: if self.isLink(obj): return False - # TODO - JD: This might have been enough way back when, but additional - # checks are needed now. + return self._isTextAreaByType(obj) + + def _isTextAreaByType(self, obj): + # NOTE: This is legacy and may need more checks now. return AXUtilities.is_text_input(obj) \ or AXUtilities.is_text(obj) \ or AXUtilities.is_paragraph(obj) @@ -2669,19 +2705,30 @@ class Utilities: msg = "SCRIPT UTILITIES: Broken text insertion event" debug.printMessage(debug.LEVEL_INFO, msg, True) - if AXUtilities.is_password_text(event.source): - text = self.queryNonEmptyText(event.source) - if text: - string = AXText.get_all_text(event.source) - if string: - tokens = ["HACK: Returning last char in '", string, "'"] - debug.printTokens(debug.LEVEL_INFO, tokens, True) - return string[-1] + fallbackText = self._getFallbackInsertedText(event) + if fallbackText: + return fallbackText msg = "FAIL: Unable to correct broken text insertion event" debug.printMessage(debug.LEVEL_INFO, msg, True) return "" + def _getFallbackInsertedText(self, event): + if not AXUtilities.is_password_text(event.source): + return "" + + text = self.queryNonEmptyText(event.source) + if not text: + return "" + + string = AXText.get_all_text(event.source) + if not string: + return "" + + tokens = ["HACK: Returning last char in '", string, "'"] + debug.printTokens(debug.LEVEL_INFO, tokens, True) + return string[-1] + def selectedText(self, obj): """Get the text selection for the given object. @@ -4600,10 +4647,9 @@ class Utilities: if AXUtilities.is_showing(obj) and AXUtilities.is_visible(obj): return True - # TODO - JD: This really should be in the toolkit scripts. But it - # 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. + # NOTE: This likely belongs in toolkit scripts, but it's shared across + # toolkits (Gtk, Firefox, Chrome, LO, Eclipse) and might be an AT-SPI2 + # issue, so we keep it here for now. if self._is_open_menu_bar_menu_role(obj): tokens = ["HACK: Treating", obj, "as showing and visible"] debug.printTokens(debug.LEVEL_INFO, tokens, True) @@ -4788,19 +4834,22 @@ class Utilities: for x in [k for k in textSelections.keys() if textSelections.get(k) == value]: textSelections.pop(x) - # TODO: JD - this doesn't yet handle the case of multiple non-contiguous - # selections in a single accessible object. start, end, string = 0, 0, '' if text: - string, start, end = AXText.get_selected_text(obj) - if string: - string = self.expandEOCs(obj, start, end) + string, start, end = self._getSingleSelectionText(obj) tokens = ["SCRIPT UTILITIES: New selection for", obj, f"is '{string}' ({start}, {end})"] debug.printTokens(debug.LEVEL_INFO, tokens, True) textSelections[hash(obj)] = start, end, string self._script.pointOfReference['textSelections'] = textSelections + def _getSingleSelectionText(self, obj): + # NOTE: Does not handle multiple non-contiguous selections. + string, start, end = AXText.get_selected_text(obj) + if string: + string = self.expandEOCs(obj, start, end) + return string, start, end + @staticmethod def onClipboardContentsChanged(*args): script = cthulhu_state.activeScript @@ -5135,9 +5184,13 @@ class Utilities: if bool(re.search(r"\w", event.any_data)) != bool(re.search(r"\w", contents)): return False - # HACK: If the application treats each paragraph as a separate object, - # we'll get individual events for each paragraph rather than a single - # event whose any_data matches the clipboard contents. + if self._isParagraphClipboardEvent(event, contents): + return True + + return False + + def _isParagraphClipboardEvent(self, event, contents): + # NOTE: Paragraph-per-object toolkits can emit per-paragraph events. if "\n" in contents and event.any_data.rstrip() in contents: return True