Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 313444

Summary:

[Accessibility] Font attributes are repeated at the beginning of lines

Product:

[Eclipse Project] Platform

Reporter:

Scott Kovatch <skovatch>

Component:

SWT

Assignee:

Scott Kovatch <skovatch>

Status:

RESOLVED FIXED

QA Contact:

Carolyn MacLeod <carolynmacleod4>

Severity:

normal

Priority:

CC:

Silenio_Quarti

Version:

3.6

Flags:

carolynmacleod4: review+
Silenio_Quarti: review+

Target Milestone:

3.6 RC2

Hardware:

Macintosh

OS:

Mac OS X

Whiteboard:

Attachments:

Description	Flags
Fix	none

Description Scott Kovatch

2010-05-18 17:19:20 EDT

I20100516-0800

When voiceover is turned on, font attributes of text will be read out at the start of a line even when the font hasn't changed over a run of text.

To reproduce: 

1. Make sure text attributes are being read out. In VoiceOver Utility, select Verbosity -> Text, and make sure 'When text attributes change:" says "Speak Attributes".
2. Run CustomControlExample.
3. Select the last word of the first line and the first word of the second line.
4. Make the text bold.
5. Click on the word "Two"
6. Click on the word "The"
===> You hear the font attribute being read out.

The font didn't change from the default, so the font attribute shouldn't have been read.

Comment 1 Scott Kovatch

2010-05-18 17:24:14 EDT

Created attachment 169028 [details]
Fix

Attribute locations set on the returned attributed string must start at zero. Code could set a negative location, depending on where the attribute started. Likewise, they can't run off the end of the string.

Comment 2 Scott Kovatch

2010-05-18 17:25:26 EDT

Ready for review. This is the extra code I accidentally checked in last week, and this was the bug it was fixing.

Comment 3 Silenio Quarti

2010-05-19 15:30:17 EDT

I do not quite understand what the code is doing, but I believe the portion of the code below is comparing offsets with lengths which cannot be right.

if (attributeRange.location + attributeRange.length > range.length) {
	attributeRange.length = range.length - attributeRange.location;
}

Shouldn't it be this instead?

if (attributeRange.location + attributeRange.length > range.location + range.length) {
	attributeRange.length = (range.location + range.length) - attributeRange.location;
}

Comment 4 Scott Kovatch

2010-05-19 16:08:21 EDT

(In reply to comment #3)
> I do not quite understand what the code is doing, but I believe the portion of
> the code below is comparing offsets with lengths which cannot be right.

getAttributedStringForRangeParameterizedAttribute needs to return an attributed string which is a substring of the entire content.  If the requested location falls in the middle of a style run we compute an attribute on the returned substring with a negative location. Likewise, if the end of the requested range falls in the middle of a run, we have to shorten the returned length so it doesn't run off the end of the substring.
 
> if (attributeRange.location + attributeRange.length > range.length) {
>     attributeRange.length = range.length - attributeRange.location;
> }

I agree, it is a bit confusing. range.location is the start location of the substring that needs to be returned, not the location in the returned string. It doesn't change.

The starting location here is 0. We're making sure the current attribute to be set would not run past the end of the returned substring.

Comment 5 Silenio Quarti

2010-05-19 16:29:46 EDT

Ok, I understand it now.

Comment 6 Carolyn MacLeod

2010-05-19 16:31:57 EDT

After wrapping 2 tired brains around the math once more, we are both good with the change in comment 1.

Comment 7 Scott Kovatch

2010-05-19 17:53:48 EDT

Thanks! Fixed > 20100519.