GNOME Bugzilla – Bug 523268
Clean up braille attributes code
Last modified: 2009-03-10 00:04:46 UTC
The current Text.getAttributeMask() method in braille.py is longer than it really needs to be. It also does some toolkit specific operations directly. These stuff should be delegated to the scripts for proper handling.
Created attachment 107576 [details] [review] Proposed patch This patch cleans up the code a bit, and delegates all the appropriate operations to the script.
Joanie - you added the braille attributes code. When you get time, can you please take a look at this patch? Delegating to the script seems like the right thing to do.
Yup, Eitan and I talked about this at CSUN. Seems to do the right thing and is a lot cleaner. Thanks Eitan!
(In reply to comment #3) > Yup, Eitan and I talked about this at CSUN. Seems to do the right thing and is > a lot cleaner. Thanks Eitan! > Cool. So...I didn't wrap my mind around the original implementation, and we don't really have any good tests for it. So...on this one...Joanie, Eitan, Mike, can you test with the new patch and get a decent feeling about the code? The patch seems good to me (modulo lint and testing). If it pylints well and tests well, I say commit. Thanks!
Created attachment 108032 [details] [review] Proposed patch This does a minor tweak in StarOffice, removes a warning about a differing method signature on an override. And thus we get a pylint score of 10.00.
OK, committed to trunk. Thanks y'all.
This patch seems to have broken the speaking of the addressbar in firefox. Open firefox and press CTRL+l to get to the address bar. Notice that you hear nothing after updating orca to the release containing this patch.
Damn. Crises mode... Obviously this was premature. I will post here once I get a fix in trunk.
Ok, I fixed this in trunk. Sorry about that. I hope I didn't introduce any other trouble.
This checkin seems to have an error in braille.py and orca will not run!
Eitan, I took a look at the error Mike reported and the patch. This was in your patch: @@ -613,7 +613,11 @@ if self.label: +<<<<<<< HEAD:src/orca/braille.py regionMask = [0]*len(self.label) + mask +======= + regionMask = [0]*len(self.label) + regionMask +>>>>>>> brl_attributes_cleanup:src/orca/braille.py return ''.join(map(chr, regionMask)) So I got rid of the bogus lines. That at least gets trunk working again. The location bar does the right thing. A very cursory glance at other things related to selection and attributes hasn't turned up any problems. But I want to stress *very* cursory.
Joanie, thanks a lot for making trunk functional again. (smile) I haven't noticed any problem with your changes yet.
Ok, so this has been in trunk for two days, and we only had two major breakages. I think it's time to close it ;)
This can't be closed because the functionality does not work in trunk.
Reopening per Mike's comments.
Reclosing per Mike's phone comments.