Trevor Saunders (:tbsaunde) Assignee
	Description • 9 years ago

      No description provided.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 4 • 9 years ago

Comment on attachment 8605942 [details] [diff] [review]
move AccessibleWrap::FireStateChangeEvent to be a member of MaiAtkObject

Review of attachment 8605942 [details] [diff] [review]:
-----------------------------------------------------------------

ok r=me. We have a comment somewhere that "MaiAtkObject is a thin wrapper"; at some point we might want to edit that.

::: accessible/atk/AccessibleWrap.cpp
@@ +1128,5 @@
>  
>      switch (type) {
>      case nsIAccessibleEvent::EVENT_STATE_CHANGE:
> +      {
> +        AccStateChangeEvent* event = downcast_accEvent(aEvent);

We used to NS_ENSURE_TRUE here which was our pattern when using QueryInterface to downcast... should we keep it?

@@ +1130,5 @@
>      case nsIAccessibleEvent::EVENT_STATE_CHANGE:
> +      {
> +        AccStateChangeEvent* event = downcast_accEvent(aEvent);
> +        MAI_ATK_OBJECT(atkObj)-> FireStateChangeEvent(event->GetState(),
> +                                                      event->IsStateEnabled());

nit: unnecessary space after the initial dereference '->'

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 5 • 9 years ago

Comment on attachment 8605946 [details] [diff] [review]
fire useful state change and caret move events for proxies

Review of attachment 8605946 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit and question.

::: accessible/base/EventQueue.cpp
@@ +503,5 @@
>          ipcDoc->SendEvent(id, aEvent->GetEventType());
>        break;
> +    case nsIAccessibleEvent::EVENT_STATE_CHANGE: {
> +        AccStateChangeEvent* event = downcast_accEvent(aEvent);
> +        ipcDoc->SendStateChangeEvent(id, event->GetState(),

Do we need to ensure event or is the downcast guaranteed?

@@ +510,5 @@
> +      }
> +    case nsIAccessibleEvent::EVENT_TEXT_CARET_MOVED: {
> +                                                       AccCaretMoveEvent* event = downcast_accEvent(aEvent);
> +                                                       ipcDoc->SendEvent(id, event->GetCaretOffset());
> +                                                       break;

The indenting here went haywire.

	Trevor Saunders (:tbsaunde) Assignee
	Comment 6 • 9 years ago

(In reply to David Bolter [:davidb] from comment #4)
> Comment on attachment 8605942 [details] [diff] [review]
> move AccessibleWrap::FireStateChangeEvent to be a member of MaiAtkObject
> 
> Review of attachment 8605942 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ok r=me. We have a comment somewhere that "MaiAtkObject is a thin wrapper";
> at some point we might want to edit that.
> 
> ::: accessible/atk/AccessibleWrap.cpp
> @@ +1128,5 @@
> >  
> >      switch (type) {
> >      case nsIAccessibleEvent::EVENT_STATE_CHANGE:
> > +      {
> > +        AccStateChangeEvent* event = downcast_accEvent(aEvent);
> 
> We used to NS_ENSURE_TRUE here which was our pattern when using
> QueryInterface to downcast... should we keep it?

It should never fail and if does I think its a bug so crashing seems better than pretending its ok.


> @@ +503,5 @@
> >          ipcDoc->SendEvent(id, aEvent->GetEventType());
> >        break;
> > +    case nsIAccessibleEvent::EVENT_STATE_CHANGE: {
> > +        AccStateChangeEvent* event = downcast_accEvent(aEvent);
> > +        ipcDoc->SendStateChangeEvent(id, event->GetState(),
> 
> Do we need to ensure event or is the downcast guaranteed?

same

	Pulsebot
	Comment 7 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9afa8ff148e
https://hg.mozilla.org/integration/mozilla-inbound/rev/661f089f73b5
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5649b9714d1

	Phil Ringnalda (:philor)
	Comment 8 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/d9afa8ff148e
https://hg.mozilla.org/mozilla-central/rev/661f089f73b5
https://hg.mozilla.org/mozilla-central/rev/e5649b9714d1