FirePlatformEvent shouldn't be virtual

RESOLVED FIXED in mozilla24

Status

()

Core
Disability Access APIs
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: surkov, Assigned: xuku)

Tracking

(Blocks: 1 bug)

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
see http://mxr.mozilla.org/mozilla-central/ident?i=FirePlatformEvent&filter=
why can't we just put all that code in HandleAccevent()?
(Reporter)

Comment 2

4 years ago
it seems ok
(Reporter)

Comment 3

4 years ago
assigning to Zach per email.
Assignee: nobody → zach.xuku
(Assignee)

Comment 4

4 years ago
Created attachment 749908 [details] [diff] [review]
Takes FirePlatformEvent() code and makes it inline at the only call site per platform, removes declarations and definitions of the function.
Attachment #749908 - Flags: review?(trev.saunders)
Comment on attachment 749908 [details] [diff] [review]
Takes FirePlatformEvent() code and makes it inline at the only call site per platform, removes declarations and definitions of the function.

> AccessibleWrap::HandleAccEvent(AccEvent* aEvent)
> {
>   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
> 
>   nsresult rv = Accessible::HandleAccEvent(aEvent);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  return FirePlatformEvent(aEvent);
>-
>-  NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
>-}
>-
>-nsresult
>-AccessibleWrap::FirePlatformEvent(AccEvent* aEvent)
>-{
>   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;

I think you can remove this and then you won't need the second one at the end of the function.
Attachment #749908 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 6

4 years ago
Created attachment 750837 [details] [diff] [review]
Updated with requested changes
Attachment #749908 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/c66d8ea1b2f1
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c66d8ea1b2f1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.