Closed Bug 773760 Opened 8 years ago Closed 8 years ago
Cross-process clipboard setting doesn't reflect privacy status of original transferable
ContentParent::RecvSetClipboard just creates a transferable with a blank privacy context. We should pass the privacy information from the original one over the wire and stuff that into the new transferable instead.
Is this something you wanna take on Josh?
http://hg.mozilla.org/mozilla-central/file/b5ae446888f5/dom/ipc/ContentParent.cpp#l689 - this is the receiver for http://mxr.mozilla.org/mozilla-central/source/dom/ipc/PContent.ipdl#282, and is sent from http://mxr.mozilla.org/mozilla-central/source/widget/android/nsClipboard.cpp#53. We need to obtain http://mxr.mozilla.org/mozilla-central/source/widget/nsITransferable.idl#175, send it in the SetClipboardText message, and add a special method to http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsTransferable.cpp (and the nsITransferable IDL) that allows us to set the privacy status of a transferable object with just a boolean argument.
I'm confused though why nsITransferable.idl doesn't have the existing method nsTransferable::GetIsPrivateData() defined in it, so I didn't include the new method I added, nsTransferable::SetIsPrivateData(), yet. Also, how would I test this? Do I need to compile Firefox for Android and test on an android phone/tablet? My patch is based entirely on the excellent explanation in comment 2 without having tried to reproduce the bug.
There is no way to test it, unfortunately. A) this code only runs in XUL Fennec, and B) XUL Fennec doesn't have a private browsing mode. However, this may end up running in B2G one day, and then we'll all be glad we fixed it.
Also, IDL attribute foo turns into GetFoo in C++ (and SetFoo if not readonly).
Comment on attachment 651266 [details] [diff] [review] I'd like to try and fix this bug; please assign to me. This is my first go at a patch for this bug. Please let me know if I'm on the right track. Great! This looks fine to me, but I'll get an official word from ehsan.
Comment on attachment 651266 [details] [diff] [review] I'd like to try and fix this bug; please assign to me. This is my first go at a patch for this bug. Please let me know if I'm on the right track. Review of attachment 651266 [details] [diff] [review]: ----------------------------------------------------------------- Hmm, I don't particularly like the addition of the SetIsPrivateData method, but I see no better way of doing this... :( I'm mostly minusing the review because of the uuid change for nsITransferable. My other comment is just a small nit. Thanks for working on this! ::: widget/nsITransferable.idl @@ +171,5 @@ > void removeDataFlavor ( in string aDataFlavor ) ; > > attribute nsIFormatConverter converter; > > + [noscript] attribute boolean isPrivateData; You need to change the uuid of the nsITransferable interface. You can use http://www.famkruithof.net/uuid/uuidgen to generate a new uuid. Also, please add a comment to isPrivateData saying that people should try to avoid calling the SetIsPrivateData method as much as they can...
Attachment #651266 - Flags: review?(ehsan) → review-
Should I put a reason in the comments for avoiding use of SetIsPrivateData()? What's the reason?
Status: NEW → ASSIGNED
The reason is that it overrides the privacy status with a value that may not reflect the status of the context in which the transferable was created.
Comment on attachment 651641 [details] [diff] [review] Incorporate review comments. Review of attachment 651641 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks!
Attachment #651641 - Flags: review?(ehsan) → review+
Target Milestone: --- → mozilla17
Sorry, had to back out for build failures: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=327d9e9efbf7 eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=14372384&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=14372342&tree=Mozilla-Inbound jdm, please can you help Eric get this fixed up and submit the patch to Try? :-) Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/06a7377ac153
Argh, I see the problem. Will fix and upload patch and ask for push to try.
Please push to try. Realize now my local compiles wouldn't have caught previous patch's bustage because I'm not set up to compile for android.
Attachment #651641 - Attachment is obsolete: true
https://tbpl.mozilla.org/?tree=Try&rev=e359c1cfb406 Sorry we didn't do this the first time round!
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.