614800 - coalesce text attribute change events and fire caret move event for the document where event occurred

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

1) coalesce text attribute change events - we get notification for each misspelled word and may fire lot of text attr change events, this is notification event only and dupes don't make any sense.
2) fire caret move event for the document where event occurred - it make sense to process events there where they happens.

This is cleaning up, fixing some stuffs while I'm around. Ah right, this bug is isolated from upcoming patch of bug 498015, the 2nd is needed for it, the 1st is just correct.

Comment 1

[Marco Zehe (:MarcoZ)]

Comment on attachment 493245 [details] [diff] [review]
patch

r=me for the test part, thanks!

Comment 2

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

Comment on attachment 493245 [details] [diff] [review]
patch

r+a=me.

>@@ -430,22 +429,19 @@ nsAccUtils::GetTextAccessibleFromSelecti

>-      if (aNode)
>-        NS_ADDREF(*aNode = accessible->GetNode());
>+    if (textAcc)
>+      return textAcc;

You removed the addref because we know there is a cached reference right?

>-nsresult
>-nsCaretAccessible::NormalSelectionChanged(nsIDOMDocument *aDoc,
>-                                          nsISelection *aSel)
>+void
>+nsCaretAccessible::NormalSelectionChanged(nsDocAccessible *aDocument,
>+                                          nsISelection *aSelection)
> {
>-  NS_ENSURE_TRUE(mRootAccessible, NS_ERROR_FAILURE);

Slightly nervous about this change (because I'm not sure I fully understand historic bug 404380). If we trust our selection listening is removed at the right time(s) then we should be fine.

>+void
>+nsCaretAccessible::SpellcheckSelectionChanged(nsDocAccessible* aDocument,
>+                                              nsISelection* aSelection)

>-
>-  nsEventShell::FireEvent(event);
>-  return NS_OK;
>+  if (event)
>+    aDocument->FireDelayedAccessibleEvent(event);

Good.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)

> >-      if (aNode)
> >-        NS_ADDREF(*aNode = accessible->GetNode());
> >+    if (textAcc)
> >+      return textAcc;
> 
> You removed the addref because we know there is a cached reference right?

because I removed it from arguments.

> >-  NS_ENSURE_TRUE(mRootAccessible, NS_ERROR_FAILURE);
> 
> Slightly nervous about this change (because I'm not sure I fully understand
> historic bug 404380). If we trust our selection listening is removed at the
> right time(s) then we should be fine.

I just moved the check to top.

Comment 4

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

(In reply to comment #3)

> > You removed the addref because we know there is a cached reference right?
> 
> because I removed it from arguments.

OK, I misread as accessible not node. Coffee time.

Comment 5

[alexander :surkov (:asurkov)]

landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/ff0e38477a8d