-
Notifications
You must be signed in to change notification settings - Fork 76
Configurable clipboard auto-clear timeout #357
base: 13
Are you sure you want to change the base?
Configurable clipboard auto-clear timeout #357
Conversation
Change-Id: Ifccaf79c15062d48fd92ef68be378507ec501246
Change-Id: Ifac2f8e32cdf4a02da8b6d4b0061fc479b261f4c
Change-Id: I263826e8c5edbabd55c3e6adc0f5a62f934d8a3f
Change-Id: Id11afc5f98113a54d8270e8b3de83355a4a88042
Change-Id: Iba45fa1e1051b6e4b403420c595e65f9a831d784
Change-Id: If559ae520c5fbfb7ff0f42980130c782afb819d0
Change-Id: I0d0975c055515ea02d82ed983f82c8aeb9f0d7e8
Change-Id: I01d8188a3a82d6083885d438a874208c487b5bb9
Change-Id: Ie45a0fb1ecddfee7dc5212e5ea30cda657640ae0
Change-Id: If4d9e8104bcc0311d051d1b0757a7dcf53c9c72d
Change-Id: Id028bae12f79d0d54d74783fcedb9fd09197db8e
Change-Id: I1f7d5134fe0152dee9a82def0be0e93dcd9b4eb8
Change-Id: I37f6e8727ec61536d4e6493e03692a7f0f9da810
Change-Id: I0c5c109edda1bf22896010a362d03049dcfbf3c9
…publish" This reverts commit fa4b2dc. Reason for revert: bugs related to conversation shortcuts can lead to system retaining shortcuts exceeding this number, causing crashes in chat apps such as whatsapp, messages ... e.t.c Change-Id: If485dc29e4f906d2ce5ec6836b3c4c47b5f21e23 (cherry picked from commit 6ed5fbe) Merged-In: If485dc29e4f906d2ce5ec6836b3c4c47b5f21e23
…877743. Change-Id: I3da13a6b59ad96d9228c0d29f715a3b450adf141
…800000957877743 into tm-qpr2-release. Change-Id: I5374518e0da2e461709f68f8a195b73fa9e4727c
Change-Id: If9ba15f5f63266ce58f32edc1e1bedeffd6865ca
Change-Id: I51af1d29d4d7e3dec7479d69b907d2c53699186f
Change-Id: I07c013ec90fe86a51c905b4aea0a3ef86409e92f
Change-Id: I352c7faee11acf2e5505955b98b78cc7ab2f5051
Change-Id: Iaa7e7c474c9b2f44c8f44638faac81256ae038ba
Change-Id: I932dbef3f918bdbef5aa66a55d56a3979165e5e5
Change-Id: I8fa1c3a800dfea422c0d931837ae5d05b7ba8133
Change-Id: Id55b5226f75c4fb6ed28fe803a8e70f6396a8bd6
Change-Id: Ia5e5757c87f079fbfd3141ad1a78175551b199d1
Change-Id: I28e9d41f91a0a192932dc61b1835e9c54c50420a
This reverts commit 5280003.
Co-authored-by: flawedworld <flawedworld@flawed.world> Signed-off-by: r3g_5z <june@girlboss.ceo>
will rebase today. |
a2e7124
to
332d28f
Compare
re-based successfully. the feature should be ready now. |
/** @hide */ | ||
public class ClipboardConstants { | ||
// From com.android.server.clipboard.ClipboardService.DEFAULT_CLIPBOARD_TIMEOUT_MILLIS | ||
public static final int DEFAULT_CLIPBOARD_TIMEOUT_MILLIS = 3600000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to declare this as a separate constant in a separate file.
It's better to use TimeUnit.HOURS.toMillis(1)
in CLIPBOARD_AUTO_CLEAR_TIMEOUT
definition (default value can likely be reduced to several minutes) and to remove constant com.android.server.clipboard.ClipboardService.DEFAULT_CLIPBOARD_TIMEOUT_MILLIS
to make sure it's not used elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just following the convention from com.android.server.clipboard.ClipboardService.DEFAULT_CLIPBOARD_TIMEOUT_MILLIS
for making it easier to port to other versions of AOSP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also includes changing CTS as this value is used there. Depends if you don't mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If constant is removed, build will fail if it's used elsewhere in a new AOSP version. This is more reliable than assuming that search for usages of this constant will be performed each time new AOSP version comes out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is to write to DeviceConfig setting directly instead of replacing it with a new setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would we make it per-user? The current implementation doesn't allow per-user configuration because it uses DeviceConfig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this setting needs to be per-user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It adds better isolation, and even security (no leaks between user profiles).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DeviceConfig settings are unreadable by unprivileged apps.
I've thought some more about this feature, and I think that it's better to just reduce this timeout to 10 or 5 minutes instead of adding one more setting. There's a large number of privacy-sensitive timeouts and flags in AOSP, making them configurable is not a right approach in most cases in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's still a good feature to add. People can copy passwords (for some reason) or other sensitive information from other apps and not worry about it existing for a long time and some app stealing it. It's also a pretty small change. Just a "setting" file and copying two constants. Can be easily added.
515ab1e
to
e541648
Compare
I stand by what I wrote previously in the review:
|
If so, we should use a small timeout like one minute. An enhancement would be resetting the timeout every time the user pastes the same clipboard. |
I think one minute is way too short. Another approach would be to deny read access to clipboard for all apps (except IME) by default, and show a notification when an app tries to access it. Basically, an off-by-default per-app toggle for clipboard access. |
Yeah one minute is short that's why I thought every time the user pastes the clipboard it would reset. But a clipboard access permission is also possible and also be easily implemented. I think both (one minute timeout or more, and...) of the approaches are good enhancements. |
I think that one minute is too short even for the first paste. |
Right it doesn't have to be one minute, I'm only clarifying that both of the approaches are good. It could be even five minutes. |
08f4f44
to
eeec109
Compare
1df6ddf
to
679fee8
Compare
257cd49
to
e5bb142
Compare
solves: GrapheneOS/os-issue-tracker#2012