394688 - setCaretOffset() is off by one character (should be 0-indexed?)

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Attachment: test case/sample document

Steps to reproduce:

1. Launch Accerciser and highlight the document frame of the attached test case.

2. Type the following in the iPython Console:

    text = acc.queryText()
    text.setCaretOffset(3)
    text.caretOffset

Expected results:  3 would be returned

Actual results:  2 is returned

Notes:

1. Looking at the position of the caret on screen, 2 is the correct offset as offsets are 0-indexed.  It seems that it's setCaretOffset() that is off by one.  

2. This was introduced in Gecko/2007080404; in 2007080308 the offsets match.

Comment 1

[Aaron Leventhal]

That's when we checked in bug 348901, so maybe we're forgetting to convert from hypertext offsets internal source offsets. Internally we need to convert it to a source offset before we use it.

Comment 2

[Aaron Leventhal]

Wait, so are we setting the correct caret position but returning the wrong offset?

Comment 3

[Joanmarie Diggs]

Nope, just the opposite.  You're returning the correct offset. setCaretOffset(x) is currently setting the offset to x-1 instead of x.

Comment 4

[Aaron Leventhal]

Are the results affected by source whitespace?

Comment 5

[Joanmarie Diggs]

As a matter of fact, they are.  :-) I added additional whitespace in the source and the results changed.  It's not a 1-to-1 correspondence, but it's definitely related.  I should have known/guessed that. :-)

Comment 6

[Aaron Leventhal]

Attachment: First attempt -- needs testing

Comment 7

[Aaron Leventhal]

Joanie, this patch works for me with that change I mentioned. I think because you build statically you have to rebuild more than mozilla/accessible -- maybe in mozilla/browser as well. I'm not sure.

Comment 8

[Aaron Leventhal]

Attachment: WIP -- don't test this yet

Comment 9

[Aaron Leventhal]

Attachment: Patch for review

Comment 10

[Aaron Leventhal]

Setting the caret to -1 will be taken care of in a different bug.

Comment 11

[Aaron Leventhal]

Please ignore the first hunk in the diff of nsHyperTextAccessible.cpp, because it was going to help deal with setting the caret to -1. I'm going to work on that separately as I said.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 279738 [details] [diff] [review]
Patch for review

Looks good (except for the GetCharacterCount hunk we discussed on IRC)