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.

Sign up for GitHub

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

Jump to bottom

use cli.canSupport to determine intentional mentions support #3445

Merged
merged 4 commits into from
Jun 6, 2023
Merged

use cli.canSupport to determine intentional mentions support #3445

merged 4 commits into from
Jun 6, 2023

Conversation

kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented Jun 6, 2023 •
edited by github-actions bot
Loading

Fixes: element-hq/element-web#25497

supportsIntentionalMentions was being set using a client option set on client start. react-sdk set this value using a setting with an unstable version controller. react-sdk's ServerSupportUnstableFeatureController uses the matrix client to determine support, and is only initialised after the client is started. So, intentional mention support in matrix-js-sdk was always falsy.

supportsIntentionalMentions is used in react-sdk during client-side notification creation.

Now, intentional mention support uses the client's canSupport map.
This means the user's lab setting does not affect supportsIntentionalMentions. Intentional mentions is moving out of labs soon after these bugs are fixed so the impact will be minimal.
Users will be impacted where:

  1. Server supports IM and
  2. Sender has IM labs enabled and
  3. Recipient has IM labs disabled and
  4. Message includes a plaintext mention of the recipient

If the sender Alice includes a plaintext mention of Bob, (ie 'Bob' without any pillification), recipient Bob may expect a ping for this message and will not get one.
If the sender includes an intentional mention of Bob, Bob will be pinged.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

🐛 Bug Fixes

@kerryarchibald kerryarchibald mentioned this pull request Jun 6, 2023
Merged
3 tasks
@kerryarchibald kerryarchibald marked this pull request as ready for review June 6, 2023 07:50
@kerryarchibald kerryarchibald requested a review from a team as a code owner June 6, 2023 07:50
t3chguy
t3chguy approved these changes Jun 6, 2023
View reviewed changes
src/client.ts Outdated Show resolved Hide resolved
@t3chguy
Update src/client.ts
1f50c5c 
Co-authored-by: Michael Telatynski <[email protected]>
@kerryarchibald kerryarchibald enabled auto-merge June 6, 2023 21:46
@kerryarchibald kerryarchibald added this pull request to the merge queue Jun 6, 2023
Merged via the queue into develop with commit f0fa4d2 Jun 6, 2023
@kerryarchibald kerryarchibald deleted the kerry/25497/mentions-client-support branch June 6, 2023 22:04
@kerryarchibald kerryarchibald mentioned this pull request Jun 7, 2023
Closed
toger5 pushed a commit that referenced this pull request Jun 7, 2023
@t3chguy @toger5
use cli.canSupport to determine intentional mentions support (#3445)
accd4a5 
* use cli.canSupport to determine intentional mentions support

* more specific comment

* Update src/client.ts

Co-authored-by: Michael Telatynski <[email protected]>

---------

Co-authored-by: Michael Telatynski <[email protected]>
toger5 added a commit that referenced this pull request Jun 7, 2023
@toger5 @robintown
@renovate @t3chguy @richvdh @davidisaaclee @ShadowJonathan @stas-demydiuk @RiotRobot @andybalaam @weeman1337
Extend stats summary with call device and user count based on room st…
60c715d 
…ate (#3424)

* send expected peer connections to posthog.
(based on roomState event)

* add tests

* change GroupCallStats initialized

* prettier

* more test and catch for promise

* seperate the participant logic in a summary extend function

Signed-off-by: Timo K <[email protected]>

* remove unused

Signed-off-by: Timo K <[email protected]>

* rename summaryStatsReportGatherer to "Reporter"
for the summary stats there is only one instance because there is only
one summary. Since we dont have a list of gatherers it this class only reports.
Hence we rename it to be a reporter.

Signed-off-by: Timo K <[email protected]>

* review

Signed-off-by: Timo K <[email protected]>

* Update src/webrtc/stats/groupCallStats.ts

Co-authored-by: Robin <[email protected]>

* revert rename

Signed-off-by: Timo K <[email protected]>

* Update all non-major dependencies (#3433)

* Update all non-major dependencies

* Remove name wrap-ansi-cjs

* Remove name string-width-cjs

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* Update definitelyTyped (#3430)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* Export FALLBACK_ICE_SERVER (#3429)

* Add an integration test for verification (#3436)

* Move existing crypto integ tests into a subdirectory

* Factor out some common bits from `crypto.spec.ts`

* Integration test for device verification

* Ignore generated file in prettier

* Always show a summary after Jest tests (#3440)

... because it is otherwise impossible to see what failed.

* Use correct /v3 prefix for /refresh (#3016)

* Add tests to ensure /v3/refresh is called + automatic /v1 retry

* Request /refresh with v3 prefix, and quietly fall back to v1

* Add tests checking re-raising errors

* Update spec/unit/login.spec.ts

* Update comment

Co-authored-by: Richard van der Hoff <[email protected]>

---------

Co-authored-by: Richard van der Hoff <[email protected]>

* Update Mutual Rooms (MSC2666) support (#3381)

* update mutual rooms support

* clarify docs and switch eslint comment with todo

* please the holy linter

* change query variable names around

* add mock tests and fix issue

* ye holy linter

* GHA: build and cypress-test a copy of element-web after each push (#3412)

* Build a copy of element-web after each push

* Run cypress after each build of element-web

* Fix downstream-artifacts build (#3443)

* Fix downstream-artifacts build

* Update cypress.yml

* Fix edge cases around 2nd order relations and threads (#3437)

* Fix tests oversimplifying threads fixtures

* Check for unsigned thread_id in MatrixEvent::threadRootId

* Fix threads order being racy

* Make Sonar happier

* Iterate

* Make sliding sync linearize processing of sync requests (#3442)

* Make sliding sync linearize processing of sync requests

* Iterate

* Iterate

* Iterate

* Iterate

* Disable downstream artifacts build for develop branch (#3444)

* Export thread-related types from SDK (#3447)

* Export thread-related types from SDK

* address PR feedback

* Integration test for QR code verification (#3439)

* Integration test for QR code verification

Followup to #3436: another
integration test, this time using the QR code flow

* Use Object.defineProperty, and restore afterwards

Apparently global.crypto exists in some environments

* apply ts-ignore

* remove stray comment

* Update spec/integ/crypto/verification.spec.ts

* Add `getShowSasCallbacks`, `getShowQrCodeCallbacks` to VerifierBase (#3422)

* Add `getShowSasCallbacks`, `getShowQrCodeCallbacks` to VerifierBase

... to avoid some type-casting

* Integration test for QR code verification

Followup to #3436: another
integration test, this time using the QR code flow

* Rename method

... it turns out not to be used quite as I thought.

* tests for new methods

* Use Object.defineProperty, and restore afterwards

Apparently global.crypto exists in some environments

* apply ts-ignore

* More test coverage

* fix bad merge

* Fix changelog_head.py script to be Python 3 compatible

* Prepare changelog for v25.2.0-rc.1

* v25.2.0-rc.1

* Fix tsconfig-build.json

* Prepare changelog for v25.2.0-rc.2

* v25.2.0-rc.2

* Fix docs deployment

* Prepare changelog for v25.2.0-rc.3

* v25.2.0-rc.3

* Prepare changelog for v25.2.0-rc.4

* v25.2.0-rc.4

* [Backport staging] Attempt a potential workaround for stuck notifs (#3387)

Co-authored-by: Andy Balaam <[email protected]>

* Prepare changelog for v25.2.0-rc.5

* v25.2.0-rc.5

* [Backport staging] Fix mark as unread button (#3401)

Co-authored-by: Michael Weimann <[email protected]>

* Prepare changelog for v26.0.0-rc.1

* v26.0.0-rc.1

* Prepare changelog for v26.0.0

* v26.0.0

* Resetting package fields for development

* use cli.canSupport to determine intentional mentions support (#3445)

* use cli.canSupport to determine intentional mentions support

* more specific comment

* Update src/client.ts

Co-authored-by: Michael Telatynski <[email protected]>

---------

Co-authored-by: Michael Telatynski <[email protected]>

* git fixup

Signed-off-by: Timo K <[email protected]>

* import updates

Signed-off-by: Timo K <[email protected]>

* dont revert enricos change

Signed-off-by: Timo K <[email protected]>

* temp rename for lowercase

* lowercase

---------

Signed-off-by: Timo K <[email protected]>
Co-authored-by: Robin <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: David Lee <[email protected]>
Co-authored-by: Jonathan de Jong <[email protected]>
Co-authored-by: Stanislav Demydiuk <[email protected]>
Co-authored-by: ElementRobot <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Michael Weimann <[email protected]>
Co-authored-by: Kerry <[email protected]>
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Feb 22, 2024
@su-ex
Merge tag 'v26.1.0' into sc
10fe11f 
* Introduce a new `Crypto.Verifier` interface, and deprecate direct access to `VerificationBase`, `SAS` and `ReciprocateQRCode` ([\matrix-org#3414](matrix-org#3414)).
* Add `rust-crypto#isCrossSigningReady` implementation ([\matrix-org#3462](matrix-org#3462)). Contributed by @florianduros.
* OIDC: Validate `m.authentication` configuration ([\matrix-org#3419](matrix-org#3419)). Contributed by @kerryarchibald.
* ElementR: Add `CryptoApi.getCrossSigningStatus` ([\matrix-org#3452](matrix-org#3452)). Contributed by @florianduros.
* Extend stats summary with call device and user count based on room state ([\matrix-org#3424](matrix-org#3424)). Contributed by @toger5.
* Update MSC3912 implementation to use `with_rel_type` instead of `with_relations` ([\matrix-org#3420](matrix-org#3420)).
* Export thread-related types from SDK ([\matrix-org#3447](matrix-org#3447)). Contributed by @stas-demydiuk.
* Use correct /v3 prefix for /refresh ([\matrix-org#3016](matrix-org#3016)). Contributed by @davidisaaclee.
* Fix thread list being ordered based on all updates ([\matrix-org#3458](matrix-org#3458)). Fixes element-hq/element-web#25522.
* Fix: handle `baseUrl` with trailing slash in `fetch.getUrl` ([\matrix-org#3455](matrix-org#3455)). Fixes element-hq/element-web#25526. Contributed by @kerryarchibald.
* use cli.canSupport to determine intentional mentions support ([\matrix-org#3445](matrix-org#3445)). Fixes element-hq/element-web#25497. Contributed by @kerryarchibald.
* Make sliding sync linearize processing of sync requests ([\matrix-org#3442](matrix-org#3442)).
* Fix edge cases around 2nd order relations and threads ([\matrix-org#3437](matrix-org#3437)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t3chguy t3chguy t3chguy approved these changes

@weeman1337 weeman1337 Awaiting requested review from weeman1337 weeman1337 is a code owner automatically assigned from matrix-org/element-web-reviewers

Assignees
No one assigned
Labels
T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Intentional Mentions testing: Pings for display name and MXID text shouldn't happen
2 participants
@kerryarchibald @t3chguy