Add a pref toggle for copying/pasting non-text/rich text in B2G clipboard

RESOLVED FIXED in Firefox 44

Status

Firefox OS
Runtime
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: boris, Assigned: boris)

Tracking

unspecified
FxOS-S9 (16Oct)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

Bug 952456 add a new feature to support non-text/rich text for B2G clipboard, but some apps (ex. SMS app) only handle plain-text. According to Bug 952456 Comment 79, we could put the functionality behind a pref for 2.5 to make sure it wouldn't break those apps on 2.5.
Depends on: 952456
Assignee: nobody → boris.chiou
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Created attachment 8668321 [details] [review]
[gaia] BorisChiou:Bug1210265 > mozilla-b2g:master
Created attachment 8668325 [details] [diff] [review]
Add pref toggle for copying/pasting only plain text on B2G (v4)

Add a new preference, clibboard.plainTextOnly.
1. On:  we only support kUnicodeMIME in SetData/GetData.
2. Off: we can support different MIME types in SetData/GetData

BTW, "copy image" is dependent on non-text/rich text support, so this
menu item is also disabled after we turn it off.
Attachment #8668320 - Attachment is obsolete: true
Comment hidden (obsolete)
Attachment #8668325 - Flags: review?(fabrice)
Attachment #8668321 - Flags: review?(timdream)
Comment on attachment 8668325 [details] [diff] [review]
Add pref toggle for copying/pasting only plain text on B2G (v4)

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

r=me with comments addressed.

::: b2g/app/b2g.js
@@ +1150,5 @@
>  pref("b2g.multiscreen.chrome_remote_url", "chrome://b2g/content/shell_remote.html");
>  pref("b2g.multiscreen.system_remote_url", "index_remote.html");
>  
> +// Clipboard only supports text/plain
> +pref("clipboard.plainTextOnly", true);

We can keep that to false on m-c. We'll switch to true once we branch if needed.

::: widget/gonk/nsClipboard.cpp
@@ +51,5 @@
>  
>    // Clear out the clipboard in order to set the new data
>    EmptyClipboard(aWhichClipboard);
>  
> +  // Bug 1210265: Use a pref to toggle rich text/non-text support

Nit: no need to put the bug number in the comment, but add a full stop at the end.

@@ +181,5 @@
>      nsRefPtr<nsClipboardProxy> clipboardProxy = new nsClipboardProxy();
>      return clipboardProxy->GetData(aTransferable, aWhichClipboard);
>    }
>  
> +  // Bug 1210265: Use a pref to toggle rich text/non-text support

nit: same as the other comment.
Attachment #8668325 - Flags: review?(fabrice) → review+
Comment hidden (obsolete)
Comment on attachment 8668325 [details] [diff] [review]
Add pref toggle for copying/pasting only plain text on B2G (v4)

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

::: b2g/app/b2g.js
@@ +1150,5 @@
>  pref("b2g.multiscreen.chrome_remote_url", "chrome://b2g/content/shell_remote.html");
>  pref("b2g.multiscreen.system_remote_url", "index_remote.html");
>  
> +// Clipboard only supports text/plain
> +pref("clipboard.plainTextOnly", true);

OK.
Attachment #8668321 - Flags: review?(timdream) → review+
Comment hidden (obsolete)
Created attachment 8668832 [details] [diff] [review]
Add a pref toggle for copying/pasting only plain text on B2G (v5, carry fabrice's r+)

Add a new preference, clibboard.plainTextOnly.
1. On:  we only support kUnicodeMIME in SetData/GetData.
2. Off: we can support different MIME types in SetData/GetData

BTW, "copy image" is dependent on non-text/rich text support, so this
menu item is also disabled after we turn the pref off.

Fix some mochitest errors.
Attachment #8668325 - Attachment is obsolete: true
Attachment #8668832 - Flags: review+
Comment hidden (obsolete)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a09ac209822
Keywords: checkin-needed
Hi Sheriff,

Could you please check in gaia and gecko parts together? Thanks.
https://github.com/mozilla-b2g/gaia/commit/e217e18b96aff349332373a3e6eedb6b8067a19a

also gecko part landed :)

Comment 16

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/f58a232fc058
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f58a232fc058
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S9 (16Oct)
Blocks: 1212188
You need to log in before you can comment on or make changes to this bug.