Closed Bug 625009 Opened 13 years ago Closed 13 years ago

text offsets don't get updated when text of first child text accessible is changed

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b10
Tracking Status
blocking2.0 --- final+

People

(Reporter: surkov, Assigned: surkov)

Details

(Keywords: access, regression, Whiteboard: [softblocker])

Attachments

(1 file)

Results in hypertext interface return wrong results.
blocking2.0: --- → ?
Summary: text offsets don't get updated when text of first text accessible is changed → text offsets don't get updated when text of first child text accessible is changed
Attached patch patchSplinter Review
Attachment #503122 - Flags: review?(bolterbugz)
Comment on attachment 503122 [details] [diff] [review]
patch

r=me.

>diff --git a/accessible/src/html/nsHyperTextAccessible.cpp b/accessible/src/html/nsHyperTextAccessible.cpp

> nsHyperTextAccessible::GetChildOffset(PRUint32 aChildIndex,
>                                       PRBool aInvalidateAfter)

I see we want to invalidate after when calling from: nsDocAccessible::FireTextChangeEventForText
It is undesirable to have a getter be able to have a side effect like this even if it is somewhat documented by having the second argument. Maybe we should file a follow up on this, to either rename the getter or separate out the updating into another function?

> 
>   PRInt32 count = mOffsets.Length() - aChildIndex;
>   if (count > 0) {
>     if (aInvalidateAfter)
>       mOffsets.RemoveElementsAt(aChildIndex, count);
> 
>     return mOffsets[aChildIndex - 1];
>   }

>+++ b/accessible/tests/mochitest/hypertext/Makefile.in

>+# The Initial Developer of the Original Code is
>+# Mozilla Foundation.
>+# Portions created by the Initial Developer are Copyright (C) 2010

2011 :)

Observation we seem have, and to be adding event tests outside of our events directory. I guess that's okay.
Attachment #503122 - Flags: review?(bolterbugz) → review+
Approving soft blocker.
blocking2.0: ? → final+
Whiteboard: [softblocker]
(In reply to comment #2)

> > nsHyperTextAccessible::GetChildOffset(PRUint32 aChildIndex,
> >                                       PRBool aInvalidateAfter)
> 
> I see we want to invalidate after when calling from:
> nsDocAccessible::FireTextChangeEventForText
> It is undesirable to have a getter be able to have a side effect like this even
> if it is somewhat documented by having the second argument. Maybe we should
> file a follow up on this, to either rename the getter or separate out the
> updating into another function?

yeah, there's something weird here. I'll file a bug.
]
> >+# Portions created by the Initial Developer are Copyright (C) 2010
> 
> 2011 :)

you know different time zones, different years :)

> Observation we seem have, and to be adding event tests outside of our events
> directory. I guess that's okay.

event queues are just a tool here, we wait for event to do a test, so that's ok.
(In reply to comment #4)

> > if it is somewhat documented by having the second argument. Maybe we should
> > file a follow up on this, to either rename the getter or separate out the
> > updating into another function?
> 
> yeah, there's something weird here. I'll file a bug.

done, bug 625255
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/de195d1171d6
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.