613857 - (getText*a11y) [meta] fix getText* behaviour

Status: NEW

(Core :: Disability Access APIs, defect)

Description

[Fernando Herrera]

Attachment: Initial look into fixing GetTextHelper using current code

Currently getText functions are returning very different results from expected.

This issue here is that nsHyperTextAccessible::GetRelativeOffset uses nsIFrame::PeekOffset to calculate those offsets, and that implementation doesn't take into account stuff needed fot getText (mostly if current offset is ok for start or we should take previous or next).

So there are 3 options:
1) Still use PeekOffset but woraround/wrap at GetRelativeOffset to match getText expected interpretation
2) Modify PeekOffset to take into account getText needs
3) Use a completely new code to calculate offsets

Option 1) should not be that hard for the WORD and CHAR case, but LINE could be more ugly because PeekOffset considers "\n" as a word
Option 2) scares me, because there is a lot of rendering code depending on that behaviour
for Option 3) we may use nsILineBreaker and nsIWordBreaker so we don't have to rewrite the whole logic

What do you think?

As an example I'm attaching a patch exploring Option 1), that get proper results for CHAR and WORD cases (1008 ok vs 152 Fails for the singleline test case, 1277 vs. 51) but haven't yet tried to fix LINE (527 ok vs. 538 fails). The remaining fails for CHAR and WORD are mostly textarea adding an extra \n and out of boundary test.

Comment 1

[alexander :surkov (:asurkov)]

Can you make your patch to contain function names, i.e. please use standard options to create a patch.

existing code makes me scary since it's not very well readable, so I'm not fan for option 1.

if we need to change layout for option 2 then it should be very careful considered, like does it come with PeekOffset propose and etc.

I'm for option 3 since it can reuse existing code, we should look if it's optimal and works as we expect for rich text.

Perhaps I would go to make new algorithm trying to keep it plain as possible.

But we need to finish all tests before taking this patch and (would be nice) before reviewing.