Community
Participate
Working Groups
In StyledText's implementation of getTextAttributes() at line 6712 (roughly) it's possible for the method to return without returning a TextStyle object in e.textStyle. On the Mac this causes VoiceOver to interpret a line ending as a font change because the character before and character after have font information returned for them. Fix is simple -- just add e.textStyle = new TextStyle(st.getFont(), st.foreground, st.background); before the return on line 6715.
Created attachment 167834 [details] Fix
Felipe, let me know what you think.
Car, you should make sure this is okay, too.
That block of code runs when the offset points to the line delimiter. Not sure if you should return the default font or the font of the preceding character. What happens when the last word in the line is bold, how does your patch perform in this case ?
(In reply to comment #4) > That block of code runs when the offset points to the line delimiter. > Not sure if you should return the default font or the font of the preceding > character. > > What happens when the last word in the line is bold, how does your patch > perform in this case ? Ah, good catch. In CustomControlExample, if I select "dog.<newline>One" it tells me that the newline is plain, but the rest of the text is bold. Getting the style of the last character works as you suggested. I'll add the patch.
Created attachment 167967 [details] Better fix
I am going to leave this one up to Felipe, but yes, the styles seem to work as expected (except for italic?)
Created attachment 168822 [details] new patch Your patch decrements offset without any test, I'm afraid you can get a -1 and layout.getStyle() will throw an expection (the case to my mind is empty line "\n"). Please try this patch, it clamps offset to range, so it should return the style of preceeding characters.
(In reply to comment #8) > Created an attachment (id=168822) [details] > new patch > > Your patch decrements offset without any test, I'm afraid you can get a -1 and > layout.getStyle() will throw an expection (the case to my mind is empty line > "\n"). > > Please try this patch, it clamps offset to range, so it should return the style > of preceeding characters. Oops -- this patch still triggers an index out of bounds at e.textStyle = layout.getStyle(Math.max(0, Math.min(offset, lineLength - 1))); at org.eclipse.swt.graphics.TextLayout.getStyle(TextLayout.java:1412) at org.eclipse.swt.custom.StyledText$13.getTextAttributes(StyledText.java:6708) at org.eclipse.swt.accessibility.Accessible.getAttributedStringForRangeParameterizedAttribute(Accessible.java:1520) at org.eclipse.swt.accessibility.Accessible.internal_accessibilityAttributeValue_forParameter(Accessible.java:1198) at org.eclipse.swt.widgets.Control.accessibilityAttributeValue_forParameter(Control.java:252) in the single blank line case you describe above. This clamps to 0, but TextLayout is throwing an exception because 0 <= 0 < 0 is false.
(Not that mine was any better -- same exception.)
(In reply to comment #10) > (Not that mine was any better -- same exception.) Okay, all we need is a lineLength > 0 before making the call. sorry, I didn't test the code.
Created attachment 168925 [details] new patch (2) Is the code right now ?
(In reply to comment #12) > Created an attachment (id=168925) [details] > new patch (2) > > Is the code right now ? Interesting.... I thought I tried to make this exact change yesterday but wasn't getting the right transition when there are multiple blank lines. This morning it's okay. +1 for the last patch.
We still need an extra +1 before we release this. Car, please review the patch in comment 12. Thank you.
I looked at this with SSQ, and in order for this patch to work, the patch in bug 313444 also needs to be installed. So, please have a look at SSQ's comment 3 in bug 313444 first, and if there's any changes, those need to be reviewed/tested first, and then both will have to go in together.
OK - this is good. I am going to +1 bug 313444 is a second. They both should go in together.
Fixed > 20100519.
