GNOME Bugzilla – Bug 487237
Isolate code to infer checkbox role from table cells
Last modified: 2008-07-22 19:32:59 UTC
There is currently code in braillegenerator.py, speechgenerator.py, and rolenames.py to check for the presence of a 'toggle' action on a table cell. This is used to help us infer the table cell really is a checkbox. This may not always work with Firefox, which might instead expose an object attribute "checkable" = "true". So...we probably should have a script method along the lines of isCheckbox or something like that that can be used by the speech and braille generators. The inferring code should probably also be ripped out of rolenames.py and managed in the script. See also bug 487230.
> The > inferring code should probably also be ripped out of rolenames.py and managed > in the script. See also bug 487230. This part has been done. For the rest of the problem, I'm still not sure of the best way to solve it. Part of me wonders what we'll need to do in the case of cells that act like combo boxes or radio buttons or sliders or whatever else these things might want to do. I'm wondering if we might make the API for this more generic, such as the following method in default.py: [role, state] = inferTableCellRole(obj) Where role is a pyatspi.ROLE_* constant and the value of state depends upon the role. If no role can be inferred, the role would be pyatspi.ROLE_TABLE_CELL and state might be none. For the particular case of check boxes, the role would be pyatspi.ROLE_CHECK_BOX and state might be a list: [label, checkState] where label is the string being presented (None if there is no string) and checkState might be None or a boolean. A boolean value would mean it's checked or unchecked and a None value would mean the state is indeterminate (i.e., partially checked). In the future, this could expand this to support radio buttons and other objects as we run into them in the wild. Overall, however, I'm not thrilled with the above. Ideally, I think I'd like to be able to infer the just role and then turn around to the speech/braille generator for that role to get the rest of the presentation. I'm not sure how easy this would be, though, since it seems we need to be able handle cases where objects might be represented by a single atomic object or by a composite object. For example, GTK+ gives us a standard checkbox as a single atomic object where the indicator and label are all part of single object. In other places (e.g., the table case we're dealing with here, Gecko, and I think maybe Java Swing), I think the check box is a composite object composed of separate label and indicator objects in the hierarchy. Anyway, this is worth thinking about and discussing more. Alternatively, we could wait until the use case exceeds one and then generalize. This strategy has worked well for us because we know we end up with a solution that solves the problems we know we have. So, for the short term, we could simply just refactor this specifically for check boxes in table cells so the speech/braille generator and to-be-fixed where am I code could all use the same method. The exact mechanics of this focused refactor would depend upon how the speech/braille generators and where am I code handle the information. The recursive calling in the speech/braille generators is a little confusing to me and I haven't wrapped my mind around exactly how they get both the label and the checkbox state in the presentation.
> Alternatively, we could wait until the use case exceeds one and then > generalize. ... As we have plenty of other bugs on our plate at the moment, and this one just rearranges the deck chairs, I'm going to leave it (and all other mixed metaphors) alone for now. Thanks.
> For the rest of the problem, I'm still not sure of the best way to solve it. > ... > Alternatively, we could wait until the use case exceeds one and then > generalize. Will, I see that this bug has got a target of 2.21.3. What exactly would you like done to fix this bug? Thanks!
How about we simply just refactor this specifically for check boxes in table cells so the speech/braille generator and to-be-fixed where am I code all use the same method. The use case is that we have three things that care if something is a checkbox and, if it is, what its state is.
Created attachment 102002 [details] [review] An attempt at a fix.
After talking to Will this morning, we have decided to close this one as WONTFIX. Having said that, I've attached an attempt to try to create a fix for this; just for the code that tries to infer whether the table cell is a checkbox. The problem here is that the speech and braille generators do different things and trying to come up with common "abstract" code is not easy. For the above patch, I moved the getting of the speech utterances and the braille regions, into the new method in default.py. The problem there is that pylint now doesn't like that the two methods that are called, start with an underscore: W0212:3401:Script.tableCellIsCheckbox: Access to a protected member _getSpeechForTableCell of a client class W0212:3404:Script.tableCellIsCheckbox: Access to a protected member _getBrailleRegionsForTableCell of a client class (It's easy to fix the pylint problem, but it suggests that this is the wrong approach). Note also that the "where am I" code doesn't even handle table cells that might be checkboxes, so currently the commonality would only be extended to two source files. Will's thought was that we would reopen and/or reinvestigate this, if/when a situation arose where we updated one of the pieces of similar code and forgot to do the other one. That seems a very reasonable compromise.