get rid of unnecessary event object creation for FireAtkStateChangeEvent

RESOLVED FIXED in mozilla28

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: surkov, Assigned: lpy)

Tracking

(Blocks 1 bug)

unspecified
mozilla28
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=:tbsaunde][lang=c++][qa-])

Attachments

(1 attachment, 2 obsolete attachments)

see http://mxr.mozilla.org/mozilla-central/source/accessible/src/atk/AccessibleWrap.cpp#989 (EVENT_FOCUS handling case in AccessibleWrap::HandleAccEvent)
Please assign this bug to me. I will fix it soon
Assignee: nobody → pylaurent1314
Posted patch bug935756.patch (obsolete) — Splinter Review
Attachment #828494 - Flags: review?(surkov.alexander)
Comment on attachment 828494 [details] [diff] [review]
bug935756.patch

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

I prefer if old method was an inline wrapper around new method, that means you will need AccessibleWrap-inl.h since AccEvent.h is not included into Accessible.h
Attachment #828494 - Flags: review?(surkov.alexander)
(In reply to alexander :surkov from comment #3)
> Comment on attachment 828494 [details] [diff] [review]
> bug935756.patch
> 
> Review of attachment 828494 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I prefer if old method was an inline wrapper around new method, that means
> you will need AccessibleWrap-inl.h since AccEvent.h is not included into
> Accessible.h

why do that you can just replace the call to FireAtkStateChangeEvent() by atk_object_notify_state_change(atkObj, ATK_STATE_FOCUSED, isEnabled) and be done with it.
(In reply to Trevor Saunders (:tbsaunde) from comment #4)

> why do that you can just replace the call to FireAtkStateChangeEvent() by
> atk_object_notify_state_change(atkObj, ATK_STATE_FOCUSED, isEnabled) and be
> done with it.

sounds good
Posted patch bug935756.patch (obsolete) — Splinter Review
v2
Attachment #829079 - Flags: review?(trev.saunders)
Attachment #828494 - Attachment is obsolete: true
Attachment #829079 - Attachment is obsolete: true
Attachment #829079 - Flags: review?(trev.saunders)
Attachment #829085 - Flags: review?(trev.saunders)
Summary: override FireAtkStateChangeEvent to avoid unnecessary accessible event creation → get rid of unnecessary event object creation for FireAtkStateChangeEvent
Attachment #829085 - Flags: review?(trev.saunders) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/280a6f8ac4d2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [good first bug][mentor=:tbsaunde][lang=c++] → [good first bug][mentor=:tbsaunde][lang=c++][qa-]
You need to log in before you can comment on or make changes to this bug.