GNOME Bugzilla – Bug 385556
Missing cells skew right padding on braille device.
Last modified: 2007-01-09 14:42:40 UTC
Please describe the problem: Missing cells skew right padding on braille device. More missed cells = larger skew. Steps to reproduce: 1. Enter a missing cell mask with at least one missing cell indicated by a zero in a string of len(displaysize) 2. Enter long string of text in Gedit 3. move caret to right until padding is encountered. 4. Note padding width Actual results: padding is skewed Expected results: padding to remain the same as when all cells are valid Does this happen every time? yes Other information:
Created attachment 78798 [details] [review] fix for Missing cells skew right padding on braille device. This patch fixes bug, however a couple issues remain. 1. The missing cell count needs to be cached. Currently the send(CMD_MISSINGCELL_COUNT) command counts the missing cells (s.count()) on each call. This may be rather expensive since this is invoked in BasicBrlPerk.getLeftSlice. The problem is how to update the count on a settings change?? 2. The Brlapi guys are at it again. The CMD_CODES are in flux so Accept/Ignore had to be bypassed. A fix should be put off until brlapi settles in. A new bug will be issued as a reminder of this issue.
> This patch fixes bug, however a couple issues remain. > 1. The missing cell count needs to be cached. Currently the > send(CMD_MISSINGCELL_COUNT) command counts the missing cells (s.count()) on > each call. This may be rather expensive since this is invoked in > BasicBrlPerk.getLeftSlice. The problem is how to update the count on a > settings change?? Invalidating and updating the cached value is really easy now with the new settings code. Look at the the Setting.addObserver method in the AEState package. You can use it to register a callback that will be invoked immediately whenever the missing cell setting is changed. > 2. The Brlapi guys are at it again. The CMD_CODES are in flux so > Accept/Ignore had to be bypassed. A fix should be put off until brlapi settles > in. A new bug will be issued as a reminder of this issue. See if you can get a response from the brlapi developers about when they think the code will settle down. You might mention we're looking to use the brlapi Python binding in our next LSR release, but are concerned about the instability of the code. I'll hold off on committing this patch until you implement the caching using the setting change observer.
Created attachment 78815 [details] [review] fix for Missing cells skew right padding on braille device. I hope this doesn't back out the new brlapi KEY_CMD changes.
Does MissingCellCount need to be a Setting object? It never needs to be user configurable, shouldn't be persisted to disk, and nothing needs to be notified when it changes value. It seems like you could get away with making it a normal attribute on the state/style object. All you need to do is update that attribute value when the CellMask changes like you're doing now.
Created attachment 79190 [details] [review] patch for "missing cell skew right padding on braille device".
Parts of this patch seem to have been already applied. I manually checked the rejected regions. Committing it, but we'll have to test if it's working correctly.