[orca] Clean up combobox generation code

[Joanmarie Diggs <joanied src gnome org>]

commit 932526f489324ddb11d3e0416807cbbb37b82bd6
Author: Joanmarie Diggs <jdiggs igalia com>
Date:   Fri Apr 10 13:47:56 2020 -0400

    Clean up combobox generation code
    
    * Move calculation of combobox value out of generator for name and
      into script utility
    * Handle calculation of combobox value in generator for value
    * Also check for valuetext object attribute in web content to save
      on calculation time
    * Make braille generation more consistent with speech generation
    * Have displayedText get strings even for objects we normally wish
      to treat as textless (such as select-only comboboxes)

 src/orca/braille_generator.py                      | 18 --------------
 src/orca/formatting.py                             | 14 ++++-------
 src/orca/generator.py                              | 28 +++++++---------------
 src/orca/script_utilities.py                       | 23 +++++++++++++-----
 .../scripts/toolkits/Chromium/script_utilities.py  |  6 +++++
 src/orca/scripts/web/script_utilities.py           |  4 ++++
 src/orca/speech_generator.py                       |  7 ++++++
 7 files changed, 46 insertions(+), 54 deletions(-)
---
diff --git a/src/orca/braille_generator.py b/src/orca/braille_generator.py
index 75490d8ce..a5ebb799f 100644
--- a/src/orca/braille_generator.py
+++ b/src/orca/braille_generator.py
@@ -414,24 +414,6 @@ class BrailleGenerator(generator.Generator):
             self._restoreRole(oldRole, args)
         return result
 
-    def _generateComboBoxTextObj(self, obj, **args):
-        """For a combo box, we check to see if the text is editable. If so,
-        then we want to show the text attributes (such as selection --
-        see bug 496846 for more details).  This will return an array
-        containing a single object, which is the accessible for the
-        text object. Note that this is different from the rest of the
-        generators, which all return an array of strings.  Yes, this
-        is a hack.
-        """
-        result = []
-        textObj = None
-        for child in obj:
-            if child and child.getRole() == pyatspi.ROLE_TEXT:
-                textObj = child
-        if textObj and textObj.getState().contains(pyatspi.STATE_EDITABLE):
-            result.append(textObj)
-        return result
-
     def _generateIncludeContext(self, obj, **args):
         """Returns True or False to indicate whether context should be
         included or not.
diff --git a/src/orca/formatting.py b/src/orca/formatting.py
index e4319801d..9512e35cc 100644
--- a/src/orca/formatting.py
+++ b/src/orca/formatting.py
@@ -170,9 +170,8 @@ formatting = {
             'unfocused': '((substring and currentLineText) or labelAndName) + roleName'
             },
         pyatspi.ROLE_COMBO_BOX: {
-            'focused': 'labelAndName + roleName + expandableState',
-            'unfocused': 'labelAndName + roleName + pause + (currentLineText + anyTextSelection or 
positionInList) + ' + MNEMONIC + ' + accelerator',
-            'basicWhereAmI': 'label + roleName + pause + name + (currentLineText + anyTextSelection or 
positionInList) + ' + MNEMONIC + ' + accelerator'
+            'focused': 'labelOrName + roleName + expandableState',
+            'unfocused': 'labelOrName + roleName + pause + value + pause + positionInList + ' + MNEMONIC + ' 
+ accelerator',
             },
         pyatspi.ROLE_COMMENT: {
             'focused': 'labelOrName + roleName',
@@ -596,13 +595,8 @@ formatting = {
             },
         #pyatspi.ROLE_COLUMN_HEADER: 'default'
         pyatspi.ROLE_COMBO_BOX: {
-            # [[[TODO: WDW - maybe pass the label into the region constructor?
-            # We could then use the cursorOffset field to indicate where the
-            # combobox starts.]]]
-            #
-            'unfocused': '((comboBoxTextObj and ([Text(comboBoxTextObj[0], asString(label), asString(eol))] \
-                                               + [Region(" " + asString(roleName))])) \
-                           or [Component(obj, asString(label + name + roleName), label and 
(len(asString(label)) + 1) or 0)])'
+            'unfocused': '[Component(obj, asString(labelOrName + value + roleName), \
+                                     labelOrName and (len(asString(labelOrName)) + 1) or 0)]'
             },
         #pyatspi.ROLE_DESKTOP_ICON: 'default'
         pyatspi.ROLE_DIAL: {
diff --git a/src/orca/generator.py b/src/orca/generator.py
index 014906aed..d21c8ee14 100644
--- a/src/orca/generator.py
+++ b/src/orca/generator.py
@@ -304,26 +304,6 @@ class Generator:
         result = []
         self._script.pointOfReference['usedDescriptionForName'] = False
         name = obj.name
-        if obj.getRole() == pyatspi.ROLE_COMBO_BOX:
-            children = self._script.utilities.selectedChildren(obj)
-            if not children:
-                try:
-                    children = self._script.utilities.selectedChildren(obj[0])
-                except:
-                    pass
-            try:
-                include = lambda x: not self._script.utilities.isStaticTextLeaf(x)
-                children = children or [child for child in obj if include(child)]
-            except:
-                pass
-            names = map(self._script.utilities.displayedText, children)
-            names = list(filter(lambda x: x, names))
-            if len(names) == 1:
-                name = names[0].strip()
-            elif len(children) == 1 and children[0].name:
-                name = children[0].name.strip()
-            elif not names and obj.name:
-                name = obj.name
         if name:
             result.append(name)
         else:
@@ -1037,6 +1017,14 @@ class Generator:
         attribute if it exists on the object.  [[[WDW - we should
         consider returning an empty array if there is no value.
         """
+
+        role = args.get('role', obj.getRole())
+        if role == pyatspi.ROLE_COMBO_BOX:
+            value = self._script.utilities.getComboBoxValue(obj)
+            if value == obj.name:
+                return []
+            return [value]
+
         return [self._script.utilities.textForValue(obj)]
 
     #####################################################################
diff --git a/src/orca/script_utilities.py b/src/orca/script_utilities.py
index 500cd2d1d..4808db024 100644
--- a/src/orca/script_utilities.py
+++ b/src/orca/script_utilities.py
@@ -546,12 +546,8 @@ class Utilities:
         if role in [pyatspi.ROLE_PUSH_BUTTON, pyatspi.ROLE_LABEL] and name:
             return name
 
-        try:
-            text = self.queryNonEmptyText(obj)
-            displayedText = text.getText(0, text.characterCount)
-        except:
-            pass
-        else:
+        if 'Text' in pyatspi.listInterfaces(obj):
+            displayedText = obj.queryText().getText(0, -1)
             if self.EMBEDDED_OBJECT_CHARACTER in displayedText:
                 displayedText = None
 
@@ -4033,6 +4029,21 @@ class Utilities:
         isComboBox = lambda x: x and x.getRole() == pyatspi.ROLE_COMBO_BOX
         return pyatspi.findAncestor(obj, isComboBox) is not None
 
+    def getComboBoxValue(self, obj):
+        if not obj.childCount:
+            return self.displayedText(obj)
+
+        entry = self.getEntryForEditableComboBox(obj)
+        if entry:
+            return self.displayedText(entry)
+
+        selected = self._script.utilities.selectedChildren(obj)
+        selected = selected or self._script.utilities.selectedChildren(obj[0])
+        if len(selected) == 1:
+            return selected[0].name or self.displayedText(selected[0])
+
+        return self.displayedText(obj)
+
     def isPopOver(self, obj):
         return False
 
diff --git a/src/orca/scripts/toolkits/Chromium/script_utilities.py 
b/src/orca/scripts/toolkits/Chromium/script_utilities.py
index d82375004..a6b5b046b 100644
--- a/src/orca/scripts/toolkits/Chromium/script_utilities.py
+++ b/src/orca/scripts/toolkits/Chromium/script_utilities.py
@@ -114,6 +114,9 @@ class Utilities(web.Utilities):
         return rv
 
     def selectedChildCount(self, obj):
+        if not obj:
+            return []
+
         count = super().selectedChildCount(obj)
         if count or "Selection" in pyatspi.listInterfaces(obj):
             return count
@@ -130,6 +133,9 @@ class Utilities(web.Utilities):
         return count
 
     def selectedChildren(self, obj):
+        if not obj:
+            return []
+
         result = super().selectedChildren(obj)
         if result or "Selection" in pyatspi.listInterfaces(obj):
             return result
diff --git a/src/orca/scripts/web/script_utilities.py b/src/orca/scripts/web/script_utilities.py
index 0f2f19d6a..c6d8f6e7e 100644
--- a/src/orca/scripts/web/script_utilities.py
+++ b/src/orca/scripts/web/script_utilities.py
@@ -3241,6 +3241,10 @@ class Utilities(script_utilities.Utilities):
 
         return self._getTag(obj) == "code" or "code" in self._getXMLRoles(obj)
 
+    def getComboBoxValue(self, obj):
+        attrs = self.objectAttributes(obj, False)
+        return attrs.get("valuetext", super().getComboBoxValue(obj))
+
     def isEditableDescendantOfComboBox(self, obj):
         if not (obj and self.inDocumentContent(obj)):
             return super().isEditableDescendantOfComboBox(obj)
diff --git a/src/orca/speech_generator.py b/src/orca/speech_generator.py
index dfd3fca76..f9552042f 100644
--- a/src/orca/speech_generator.py
+++ b/src/orca/speech_generator.py
@@ -1509,6 +1509,13 @@ class SpeechGenerator(generator.Generator):
     #                                                                   #
     #####################################################################
 
+    def _generateValue(self, obj, **args):
+        result = super()._generateValue(obj, **args)
+        if result:
+            result.extend(self.voice(DEFAULT))
+
+        return result
+
     def _generatePercentage(self, obj, **args ):
         """Returns an array of strings (and possibly voice and audio
         specifications) that represents the percentage value of the