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

MSC3676: Transitioning away from reply fallbacks #3676

Merged
merged 7 commits into from
Feb 2, 2022
Merged

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Jan 26, 2022

@ara4n ara4n added the proposal A matrix spec change proposal label Jan 26, 2022
@turt2live turt2live added client-server Client-Server API kind:core MSC which is critical to the protocol's success proposal-in-review labels Jan 26, 2022
@turt2live
Copy link
Member

(not applying needs-implementation because this doesn't seem to need one)

ara4n added a commit that referenced this pull request Jan 26, 2022
Rather than relying on fallbacks being removed outright (and all the notification complications that causes in #2781), instead depend on #3676 which makes fallbacks best effort.
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

seems pretty sensible to me.

proposals/3676-transitioning-away-from-reply-fallbacks.md Outdated Show resolved Hide resolved
proposals/3676-transitioning-away-from-reply-fallbacks.md Outdated Show resolved Hide resolved
proposals/3676-transitioning-away-from-reply-fallbacks.md Outdated Show resolved Hide resolved
proposals/3676-transitioning-away-from-reply-fallbacks.md Outdated Show resolved Hide resolved
@ara4n
Copy link
Member Author

ara4n commented Jan 26, 2022

This seems to be generally accepted as an appropriate compromise until we land #2781, so i'm going to try to get it out of the way so #3440 can proceed.

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Jan 26, 2022

Team member @ara4n has proposed to merge this. The next step is review by the rest of the tagged people:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. and removed proposal-in-review labels Jan 26, 2022
@turt2live turt2live self-requested a review January 26, 2022 19:23
@mscbot
Copy link
Collaborator

mscbot commented Jan 28, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Jan 28, 2022
@codegod100
Copy link

🔔 This is now entering its final comment period, as per the review above. 🔔

Does this auto merge when a specific time period has elapsed?

@turt2live turt2live merged commit 2cd2a71 into main Feb 2, 2022
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Feb 2, 2022
turt2live added a commit that referenced this pull request Mar 9, 2022
* Threading  via  relation

* Add explainer on how to handle m.in_reply_to

* Clarify wording on threading MSC

* Mention MSC3051 in the alternative section of MSC3440

* Clarify updates to MSC2675 for MSC3440

Co-authored-by: Patrick Cloke <[email protected]>

* Line wrap the MSC

* More line wrapping for MSC3440

* Clarify single-layer event aggregation section

* Update thread-as-rooms advantages

* Clarify backwards compatibility and incremental support

* Clarify wording and correct typos

* Splitting Cerulean and MSC2836 in alternatives section

* Add dependencies for threads MSC

* Clarify intro to threads as rooms

* Add currentUserParticipated flag

* snake_case over camelCase

* Adding dependency to MSC3567

* Add threads capability

* Fix typo

* Update syntax highlighting to use jsonc

* Add limitations when fetching thread content by relation type

* Add reply chain fallback via m.in_reply_to

* Clarity in wording and fix typo

Co-authored-by: James Salter <[email protected]>
Co-authored-by: Matthew Hodgson <[email protected]>

* Cosmetic changes based on pull request feedback

* Add note to allow clients to omit fallback for rich replies

* fix typo

* Clarify wording to not confuse thread answers with quote-replies

* move relations justification to alternatives section

* Clarify handling of m.in_reply_to missing rel_type:m.thread

* Fix typo

* Fix typo

* Declare MSC2781 as a dependency

* Use rich reply over quote reply

* Depend on MSC3676 rather than MSC2781

Rather than relying on fallbacks being removed outright (and all the notification complications that causes in #2781), instead depend on #3676 which makes fallbacks best effort.

* Remove full stop typo

Co-authored-by: Erik Johnston <[email protected]>

* Clarify new filtering parameters.

* Fix typo.

* Update wording for client side considerations

Co-authored-by: Hubert Chathi <[email protected]>

* Add m.in_reply_to mixin to thread fallback

* Add guidance for clients and servers for thread invalid relations

* update thread root wording

* Add better definition to reply target event

Co-authored-by: Travis Ralston <[email protected]>

* Add note regarding forward compatibility

* link to MSC2674

* Update proposals/3440-threading-via-relations.md

Co-authored-by: Hubert Chathi <[email protected]>

* Clarification on responsibilities for the reply fallback

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

* Update `/messages` API endpoint version on example

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

* Apply wording suggestions from code review

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

* Add notes on server-side invalid relation filtering

* Fix typo

* reword paragraph about forwarding m.thread relation

* Add unstable prefix for capability endpoint

* Re-order alternatives to match intro paragraph

* rework relation_senders and relation_types definition

* Apply wording suggestions from code review

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

* Clarify fallback mechanism

* Rename filter property names

* Change m.render_in to m.display_reply_fallback

* Clarify what endpoints support the new filter

* Switch from /capabilities to /versions

* remove references to Cerulean

* Update latest_event description

* Clarity in wording and fix typo

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

* rename m.display_reply_fallback to hide_reply

* remove redundant paragraph about forward compat

* Improve bundled relationship example

* Explain context on why a thread-unaware client might want to send m.thread

* Clarify `hide_reply`

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

* Rename hide_reply to show_reply

* rename show_reply to is_falling_back

* Add note about stable support.

* Update proposals/3440-threading-via-relations.md

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

Co-authored-by: Patrick Cloke <[email protected]>
Co-authored-by: James Salter <[email protected]>
Co-authored-by: Matthew Hodgson <[email protected]>
Co-authored-by: Erik Johnston <[email protected]>
Co-authored-by: Patrick Cloke <[email protected]>
Co-authored-by: Hubert Chathi <[email protected]>
Co-authored-by: Travis Ralston <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Kim Brose <[email protected]>
@turt2live turt2live self-assigned this May 5, 2022
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1062

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels May 27, 2022
@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Jun 8, 2022
@turt2live
Copy link
Member

Merged 🎉

ichthyosaurus added a commit to ichthyosaurus/hydrogen-web that referenced this pull request Dec 5, 2024
Due to invalid formatting, replies to replies became garbled,
causing display issues in some clients. Hydrogen itself managed
to display the replies correctly but other clients and bridges
struggled because they were actually using the fallbacks.

Current spec: https://spec.matrix.org/v1.12/client-server-api/#fallbacks-for-rich-replies

Reply fallbacks are actively being removed in the upcoming spec but
that doesn't mean that Hydrogen should keep the old bugged code in place.

Upcoming MSCs:
- matrix-org/matrix-spec-proposals#2781
- matrix-org/matrix-spec-proposals#3676
- spec: matrix-org/matrix-spec#1994

Signed-off-by: Mirian Margiani <[email protected]>
b100dian pushed a commit to hydrogen-sailfishos/hydrogen-web that referenced this pull request Dec 8, 2024
Due to invalid formatting, replies to replies became garbled,
causing display issues in some clients. Hydrogen itself managed
to display the replies correctly but other clients and bridges
struggled because they were actually using the fallbacks.

Current spec: https://spec.matrix.org/v1.12/client-server-api/#fallbacks-for-rich-replies

Reply fallbacks are actively being removed in the upcoming spec but
that doesn't mean that Hydrogen should keep the old bugged code in place.

Upcoming MSCs:
- matrix-org/matrix-spec-proposals#2781
- matrix-org/matrix-spec-proposals#3676
- spec: matrix-org/matrix-spec#1994

Signed-off-by: Mirian Margiani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:core MSC which is critical to the protocol's success merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants