Closed Bug 1140917 Opened 5 years ago Closed 5 years ago

IPC Proxy for replace/insert/copy/cut/delete/paste

Categories

(Core :: Disability Access APIs, defect)

36 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(2 files)

Attached patch v1Splinter Review
No description provided.
Attachment #8574469 - Flags: review?(tbsaunde+mozbugs)
Comment on attachment 8574469 [details] [diff] [review]
v1

I'm pretty uncomfortable with this being sync, I'm pretty sure some of these can cause the child to execute js, but on the other hand I guess that's not really different from calling them on an object in the parent and then js cpows doing sync requests to the child.  I think its probably a mistake these are doc'd to be sync in atk, but given that I'm not sure what we can do other than this.

>+ProxyAccessible::ReplaceText(const nsAString& aText)
>+{
>+  unused << mDoc->SendReplaceText(mID, nsString(aText));

just pass const nsString&?

>+ProxyAccessible::InsertText(const nsAString& aText, int32_t aPosition)
>+{
>+  unused << mDoc->SendInsertText(mID, nsString(aText), aPosition);

same
Attachment #8574469 - Flags: review?(tbsaunde+mozbugs) → review+
Attached patch v2Splinter Review
Comment on attachment 8574845 [details] [diff] [review]
v2

>diff --git a/accessible/ipc/DocAccessibleChild.h b/accessible/ipc/DocAccessibleChild.h
>--- a/accessible/ipc/DocAccessibleChild.h
>+++ b/accessible/ipc/DocAccessibleChild.h
>@@ -157,16 +157,38 @@ public:
> 
>   virtual bool RecvScrollSubstringToPoint(const uint64_t& aID,
>                                           const int32_t& aStartOffset,
>                                           const int32_t& aEndOffset,
>                                           const uint32_t& aCoordinateType,
>                                           const int32_t& aX,
>                                           const int32_t& aY) MOZ_OVERRIDE;
> 
>+  virtual bool RecvReplaceText(const uint64_t& aID,
>+                               const nsString& aText);
>+
[etc]

All these new functions added to DocAccessibleChild should've been MOZ_OVERRIDE, BTW. The lack of these annotations here causes -Winconsistent-missing-override build warnings in clang 3.6/3.7 (which are treated as errors in warnings-as-errors builds).

I'll push a trivial followup to add those annotations here, with blanket r+ that ehsan granted me for these sorts of fixes over in 1126447 comment 2.
https://hg.mozilla.org/mozilla-central/rev/737f554af428
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Thanks. I don't think I got any warnings locally, so easy to miss.
Sure! Yeah, you'll only hit these warnings with clang 3.6 and newer.
You need to log in before you can comment on or make changes to this bug.