Skip to content

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Jun 4, 2025

Content

Adds a new developer flag, enabling our experimental support for MSC4268.

Motivation and context

Part of element-hq/element-meta#2871.

Requires matrix-org/matrix-rust-sdk#5141, so won't build yet.

Screenshots / GIFs

image

Tests

  • Enable labs flag
  • Restart app
  • In an E2EE room with history_visibility: shared (NB that this excludes rooms created on EX), send a message, and invite a user on the multiverse test client
  • On the test client, join the room, and see E2EE history 🎉

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 15

Checklist

  • Changes have been tested on an Android device or Android emulator with API 24
  • UI change has been tested on both light and dark themes
  • Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • You've made a self review of your PR

@richvdh richvdh added the PR-Feature For a new feature label Jun 4, 2025
@mxandreas
Copy link
Contributor

@richvdh Here are a suggestions for a couple of sections. I think it is important to mention that we share/accept the keys only when room's history visibility is shared. The rest is editorial.

Share encrypted history with other users when sending an invite

Share encrypted history with new members

When inviting another user to an encrypted room, share encrypted history with that user, and accept encrypted history from other users.

When inviting a user to an encrypted room that has history visibility set to shared, share encrypted history with that user, and accept encrypted history when you are invited to such a room.

@richvdh
Copy link
Member Author

richvdh commented Jun 5, 2025

Have adjusted the wording and updated the screenshot.

Copy link
Contributor

github-actions bot commented Jun 11, 2025

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/xbm4cD

Copy link

codecov bot commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.30%. Comparing base (df33cf2) to head (e64cb0c).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4821   +/-   ##
========================================
  Coverage    80.29%   80.30%           
========================================
  Files         2152     2152           
  Lines        57177    57184    +7     
  Branches      7199     7199           
========================================
+ Hits         45913    45919    +6     
  Misses        8821     8821           
- Partials      2443     2444    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@richvdh richvdh marked this pull request as ready for review June 17, 2025 14:41
@richvdh richvdh requested a review from a team as a code owner June 17, 2025 14:41
@richvdh richvdh requested review from ganfra and removed request for a team June 17, 2025 14:41
Copy link
Member

@ganfra ganfra left a comment

Choose a reason for hiding this comment

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

Just small remark.

" share encrypted history with that user, and accept encrypted history when you are invited to such a room." +
"\nRequires an app restart to take effect." +
"\n\nWARNING: this feature is EXPERIMENTAL and not all security precautions are implemented. Do not enable on" +
" production accounts.",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe split the string at the beginning of the phrase instead?
""\n\nWARNING: this feature is EXPERIMENTAL and not all security precautions are implemented." + "Do not enable on production accounts."

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, thanks. done.

Copy link
Member

Choose a reason for hiding this comment

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

The "Kotlin way" to do that is something like:

        description = """
            When inviting a user to an encrypted room that has history visibility set to "shared",
            share encrypted history with that user, and accept encrypted history when you are invited to such a room.
            Requires an app restart to take effect.

            WARNING: this feature is EXPERIMENTAL and not all security precautions are implemented.
            Do not enable on production accounts.
            """.trimIndent(),

but sadly it also adds a line creak between set to "shared", and the next line, and we cannot put all the sentence in a single line as it breaks our line lengh limit.

Ugly alternative is could be

        description = """When inviting a user to an encrypted room that has history visibility set to "shared",""" +
            " share encrypted history with that user, and accept encrypted history when you are invited to such a room."
                .let { firstLine ->
                    """
                    $firstLine
                    Requires an app restart to take effect.

                    WARNING: this feature is EXPERIMENTAL and not all security precautions are implemented.
                    Do not enable on production accounts.
                    """.trimIndent()
                },

So... just ignore my comment!

Copy link
Member

Choose a reason for hiding this comment

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

        description = """When inviting a user to an encrypted room that has history visibility set to "shared", """ +
            """
            share encrypted history with that user, and accept encrypted history when you are invited to such a room.
            Requires an app restart to take effect.

            WARNING: this feature is EXPERIMENTAL and not all security precautions are implemented.
            Do not enable on production accounts.
            """.trimIndent(),

works too.

Copy link
Member Author

Choose a reason for hiding this comment

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

To be honest, I just did the same as some of the other options already in this file. I agree it's not beautiful, but it's not awful and it is consistent.

Do you want me to update this further, or are you happy with it as-is?

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to assume it's ok.

Copy link

@richvdh richvdh merged commit b6680ac into develop Jun 18, 2025
30 of 31 checks passed
@richvdh richvdh deleted the rav/history_sharing_labs branch June 18, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-Feature For a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants