Closed Bug 614800 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: surkov, Assigned: surkov)

Details

(Keywords: access)

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #493245 - Flags: review?(marco.zehe)
Attachment #493245 - Flags: review?(bolterbugz)
Attachment #493245 - Flags: approval2.0?
Comment on attachment 493245 [details] [diff] [review]
patch

r=me for the test part, thanks!
Attachment #493245 - Flags: review?(marco.zehe) → review+
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.
Attachment #493245 - Flags: review?(bolterbugz)
Attachment #493245 - Flags: review+
Attachment #493245 - Flags: approval2.0?
Attachment #493245 - Flags: approval2.0+
(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.
(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.
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/ff0e38477a8d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.