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

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v1

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=956974f840e0

Comment 2

[Trevor Saunders (:tbsaunde)]

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

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v2

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/737f554af428

Comment 5

[Daniel Holbert [:dholbert]]

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.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/737f554af428

Comment 7

[Daniel Holbert [:dholbert]]

Landed MOZ_OVERRIDE followup described in comment 5:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/9e3bb49db815

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Thanks. I don't think I got any warnings locally, so easy to miss.

Comment 9

[Daniel Holbert [:dholbert]]

Sure! Yeah, you'll only hit these warnings with clang 3.6 and newer.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9e3bb49db815