Closed Bug 445677 Opened 13 years ago Closed 11 years ago

optimization for text attributes offsets (do not look into embedded characters)

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

(Keywords: access)

Attachments

(1 file)

spun off bug 345759 comment #159, 345759 comment #161

Should it stop early even when things are equal, but we get past the original
end's node?

Do we need to walk the whole subtree each time? Or if something has an embedded
object character can we not walk into that?
I'm sure we don't need to walk into the subtrees for embedded object chars, because the accessible for that needs to specify it's own text attributes anyway.

What I would do is, for
abc*def   
if abc and def use the same attributes, treat the whole thing as one range. Don't look inside the *.
if abc and def use diferent attributes, then you probably can just include it as part of the abc range. However you may want something more complex in that situation. It might be good to ask the screen reader developers on that one.
Keywords: access
For embedded object chars, just do what's simplest in the code. They can be part of an adjacent range. The AT will need to look inside of it anyway, if there is an accessible text child, to get the attributes for the text it represents.

Each accessible text exposes the complete set of attributes necessary to understand it.
Going forwards will be easy.

Going backwards:
Using GetPreviousSibling() is bad because O(n) in loop for GetPreviousSibling() will be O(n^2).
Instead:
1. Transform offset to accessible
2. As you go forward from there until current node, just keep track of which child was the last one with different text attributes, and the start offset for it. That will be the new start offset when the loop is finished.
Right?
For #1, I mean startOffset that was passed in.
As a general rule:
1. Each text accessible can be processed without looking up the ancestor chain
2. It is ok if an embedded object char with different text attributes in descendants can just correct that in the descendants themselves. The original text with the embedded char can just ignore the details of the descendants.

NVDA says they are fine with this. We should talk to other screen reader vendors to make sure.

(If it isn't it won't be hard to change later. If we ever need to change #2 and we wanted to break the range on embedded object chars with a difference in the subtree, then we'd need to walk the subtree in just that case. We'd probably create something like PRBool IsConsistentlyFormatted(nsIAccessible *aStartSubtree, nsTextComparer *aComparer) or something like that. If PR_TRUE we wouldn't need to break the range, otherwise we would.)


Depends on: 342045
Blocks: cleana11y
Blocks: 445516
Ping. :-)
(In reply to comment #0)
> spun off bug 345759 comment #159, 345759 comment #161
> 
> Should it stop early even when things are equal, but we get past the original
> end's node?

I don't remember where but that was fixed.

> Do we need to walk the whole subtree each time? Or if something has an embedded
> object character can we not walk into that?

changing bug summary for this item
Summary: optimization for text attributes offsets → optimization for text attributes offsets (do not look into embedded characters)
Attached patch patchSplinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Comment on attachment 446947 [details] [diff] [review]
patch

I think it's worth to take this patch not depending on performance tests from Orca. Here we started an accessible tree traversal instead a DOM tree to find text attributes and this won't be changed if algorithm is modified in the end, we'll just change its implementation slightly.
Attachment #446947 - Flags: review?(marco.zehe)
Attachment #446947 - Flags: review?(bolterbugz)
Comment on attachment 446947 [details] [diff] [review]
patch

>+  // 1. Get attribute and its ranges each after other.

Correction:
+  // 1. Get each attribute and its ranges one after another.

Very nice, thanks! r=me
Attachment #446947 - Flags: review?(marco.zehe) → review+
landed on 1.9.3 with Marco's comment - http://hg.mozilla.org/mozilla-central/rev/537743b069d0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #446947 - Flags: review?(bolterbugz)
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.