Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add privacy setting to disable personalized learning by the keyboard #6633

Merged
merged 4 commits into from
Oct 4, 2022
Merged

Add privacy setting to disable personalized learning by the keyboard #6633

merged 4 commits into from
Oct 4, 2022

Conversation

Benjiko99
Copy link
Contributor

@Benjiko99 Benjiko99 commented Jul 24, 2022

Type of change

  • Feature

Content

A new option has been added to the Privacy settings, which requests that the keyboard should not update any personalized data such as typing history and dictionary based on what the user typed.

Motivation and context

Requesting that the keyboard minimize the amount of data it records about your private communications seems sensible for an encrypted messaging app.

It should disable the keyboard's personalized learning features, like adding your typed words into the personalized dictionary or storing the history of what you type. Some keyboards may not respect this setting. Signal messenger has this feature and missed it here.

Screenshots

Tests

  • Use a keyboard that supports incognito mode, like Gboard
  • In Element, enable "Incognito keyboard" in Settings->Security & Privacy->Other
  • Enter a chat, focus the message input field, and the keyboard (in case of Gboard) will show an incognito icon in the top left corner

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 7 (24), Android 12 (31)

Checklist

@Benjiko99 Benjiko99 marked this pull request as ready for review July 24, 2022 15:55
@Benjiko99 Benjiko99 changed the title [Feature] Add privacy setting to disable personalized learning by the keyboard Add privacy setting to disable personalized learning by the keyboard Jul 24, 2022
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label Jul 25, 2022
@bmarty bmarty self-requested a review July 25, 2022 08:25
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this Pull Request. 2 remarks, else LGTM!

@@ -1522,6 +1522,8 @@ class TimelineFragment @Inject constructor(

observerUserTyping()

composerEditText.setUseIncognitoKeyboard(vectorPreferences.useIncognitoKeyboard())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid that the imeOptions will be erased by the line 1530 just below, if vectorPreferences.sendMessageWithEnter() returns true.

Maybe on the same format than setUseIncognitoKeyboard create a new fun to (properly!) handle vectorPreferences.sendMessageWithEnter() setting.

https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt#L1528

@@ -2539,6 +2539,9 @@
<string name="settings_security_prevent_screenshots_title">Prevent screenshots of the application</string>
<string name="settings_security_prevent_screenshots_summary">Enabling this setting adds the FLAG_SECURE to all Activities. Restart the application for the change to take effect.</string>

<string name="settings_security_incognito_keyboard_title">Incognito keyboard</string>
<string name="settings_security_incognito_keyboard_summary">Request that the keyboard should not update any personalized data such as typing history and dictionary based on what the user typed. Some keyboards may not respect this setting.</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the user is also the reader, it could be changed to

Suggested change
<string name="settings_security_incognito_keyboard_summary">Request that the keyboard should not update any personalized data such as typing history and dictionary based on what the user typed. Some keyboards may not respect this setting.</string>
<string name="settings_security_incognito_keyboard_summary">"Request that the keyboard should not update any personalized data such as typing history and dictionary based on what you've typed. Notice that some keyboards may not respect this setting."</string>

Also I am wondering if we should mention that the setting will only applies to the message composer (and not for instance when editing a room topic, etc.). Since other entered texts are not e2e encrypted, this is maybe fine.

We could have

Suggested change
<string name="settings_security_incognito_keyboard_summary">Request that the keyboard should not update any personalized data such as typing history and dictionary based on what the user typed. Some keyboards may not respect this setting.</string>
<string name="settings_security_incognito_keyboard_summary">"When typing messages, request that the keyboard should not update any personalized data such as typing history and dictionary based on what you've typed. Notice that some keyboards may not respect this setting."</string>

Another option would be to move this setting to the Settings/Preferences/Message editor section to mitigate this, but I think it's better to keep it in the Security & Privacy section.

@kojid0
Copy link
Contributor

kojid0 commented Sep 7, 2022

Ping @Benjiko99 :-)

@Benjiko99
Copy link
Contributor Author

All issues should be resolved now.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the update!

@bmarty bmarty enabled auto-merge October 4, 2022 15:37
@bmarty bmarty merged commit 4974fdf into element-hq:develop Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants