GNOME Bugzilla – Bug 458142
flat-review does not review Swing panels with titled borders
Last modified: 2008-07-22 19:28:11 UTC
See http://bugzilla.gnome.org/show_bug.cgi?id=455337#c3 Flat-review does not review JPanels with TitledBorders in the SwingSet2 JSlider demo. Each slider is contained in a JPanel with a TitledBorder. The border title (e.g., "major ticks") is the same as the name of the JSlider contained in the panel. Questions: 1. Do I understand correctly that Orca should treat a Swing container with a TitledBorder as part of the speech context for an object inside the container? 2. The ValueZone.__getattr__ method does not does not include the object label or name in the "string" or "brailleString". Why is this since speechgenerator._getDefaultSpeech returns the [ label name role availability ]? Orca would speak the JSlider label and name if "string" contained them.
Created attachment 91999 [details] [review] proposed patch The problem is that speechgenerator.getSpeechContext requires that the parent object has a text attribute before it will test for whether the parent has displayed text. The default.Script.getDisplayedText method does not require the object to have a text attribute. It will return the object name if it exists. The fix is to replace the getSpeechContext test on line 1658 from if not text and parent.text: with if not text:
> 1. Do I understand correctly that Orca should treat a Swing container with a > TitledBorder as part of the speech context for an object inside the container? You are correct. :-) > 2. The ValueZone.__getattr__ method does not does not include the object label > or name in the "string" or "brailleString". Why is this since > speechgenerator._getDefaultSpeech returns the [ label name role availability ]? > Orca would speak the JSlider label and name if "string" contained them. The assumption behind this is that the label will be a completely separate object from the slider. As such, the label will be represented by a completely different Zone, much as we do with text entry areas. Does Java end up including the label with the JSlider in such a way that it's not a JLabel or some other component?
> The fix is to replace the getSpeechContext test on line 1658 from > > if not text and parent.text: > > with > > if not text: This is indeed an interesting patch as it will fix this bug and bug 455354 (the patches submitted for both appear to be identical). My concern is that this might introduce undesirable things to be spoken since getDisplayedText will end up looking to obj.name under some circumstances. If obj.name of a parent is set to a non-empty string, we'll end up reading it. I realize the regression suite still is not repeatable, but it is getting better. I have a couple more changes to check in (Monday AM, I hope). After that, can you please run the tests before/after your patch and look at the diffs? The best thing to do before each runall.sh is to reboot your machine to help try to reduce the variability. If the patch doesn't introduce any of the diffs, I think the patch is probably OK to check in. It would also nice to get Mike to hammer on it for a bit as well.
One possibility is to define a new speechgenerator (that extends speechgenerator.py) for J2SE_access_bridge.py. The getSpeechContext in the new speechgenerator would use the patch above. This would remove the possibility that undesirable things would be spoke for non-Swing components. It would be relatively straight forward to verify that undesirable things are not spoke for Swing components. And yes, I'll run the regression tests before and after applying the patches. Do you want the diffs to added as a comment to the bug reports?
> One possibility is to define a new speechgenerator (that extends > speechgenerator.py) for J2SE_access_bridge.py. The getSpeechContext in the new > speechgenerator would use the patch above. This would remove the possibility > that undesirable things would be spoke for non-Swing components. It would be > relatively straight forward to verify that undesirable things are not spoke for > Swing components. This is definitely an interesting possibility. The unfortunate thing is that getSpeechContext is not easily overridable without duplicating almost the whole method. That makes the maintainability of this stuff go up. :-( If the right solution is to just do the patch to speechgenerator.py, then we should pursue that. > And yes, I'll run the regression tests before and after applying the patches. > Do you want the diffs to added as a comment to the bug reports? There might be a number of diffs. Instead of adding them as a comment, can you add them as an attachment and then providing your analysis of them as the comment for the attachment?
*** Bug 455354 has been marked as a duplicate of this bug. ***
Created attachment 92407 [details] [review] revised proposed patch The proposed patch is to override speechgenerator.getSpeechContext in J2SE_access_bridge.py. It was decided to not modify speechgenerator.getSpeechContext because of the risk of introducing inappropriate speech for non-Java components.
Looks good. Thanks Lynn!