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

Spec spoilers and color attribute allowance #2549

Closed
wants to merge 3 commits into from
Closed

Spec spoilers and color attribute allowance #2549

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27db337
Spec spoilers and color attribute allowance
turt2live May 15, 2020
7a40d8f
Change quotes to make JSON happy
turt2live May 15, 2020
01c53ef
Clarify that clients MUST send HTML for spoilers
turt2live May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/client_server/newsfragments/2548.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for spoilers and the ``color`` attribute per `MSC2010 <https://github.com/matrix-org/matrix-doc/pull/2010>`_ and `MSC2422 <https://github.com/matrix-org/matrix-doc/pull/2422>`_.
52 changes: 50 additions & 2 deletions specification/modules/instant_messaging.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ for the tag.


:``font``:
``data-mx-bg-color``, ``data-mx-color``
``data-mx-bg-color``, ``data-mx-color``, ``color``
turt2live marked this conversation as resolved.
Show resolved Hide resolved

:``span``:
``data-mx-bg-color``, ``data-mx-color``
``data-mx-bg-color``, ``data-mx-color``, ``data-mx-spoiler`` (see `spoilers <#spoiler-messages>`_)

:``a``:
``name``, ``target``, ``href`` (provided the value is not relative and has a scheme
Expand Down Expand Up @@ -499,6 +499,54 @@ the output looks similar to the following::
For ``m.image``, the text should be ``"sent an image."``. For ``m.video``, the text
should be ``"sent a video."``. For ``m.audio``, the text should be ``"sent an audio file"``.

Spoiler messages
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: Mention that spoilers are only possible on m.room.message (specifically m.text) events.

If another MSC does show up, it'd be good to expand this to textual types in general (emotes, notices, etc).

~~~~~~~~~~~~~~~~

Parts of a message can be hidden visually from the user through use of spoilers. This does
not affect the server's representation of the event content - it is simply a visual cue to
the user that the message may reveal important information about something, spoiling any
relevant surprise.

Spoilers require use of the ``formatted_body`` and therefore the ``org.matrix.custom.html``
message format, described above.
KitsuneRal marked this conversation as resolved.
Show resolved Hide resolved
turt2live marked this conversation as resolved.
Show resolved Hide resolved

Spoilers themselves are contained within ``span`` tags, with the reason (optionally) being
in the ``data-mx-spoiler`` attribute. Spoilers without a reason must at least specify the
attribute, though the value may be empty/undefined.

An example of a spoiler is:

.. code-block:: json

{
"msgtype": "m.text",
"format": "org.matrix.custom.html",
"body": "Alice [Spoiler](mxc://example.org/abc123) in the movie.",
"formatted_body": "Alice <span data-mx-spoiler>lived happily ever after</span> in the movie."
}


If a reason were to be supplied, it would look like:

.. code-block:: json

{
"msgtype": "m.text",
"format": "org.matrix.custom.html",
"body": "Alice [Spoiler for health of Alice](mxc://example.org/abc123) in the movie.",
"formatted_body": "Alice <span data-mx-spoiler="health of alice">lived happily ever after</span> in the movie."
turt2live marked this conversation as resolved.
Show resolved Hide resolved
}


The plain text (``body``) fallback for spoilers is a little different than the HTML-formatted
message as the ``body`` is often included as-is in notifications to the user. To prevent spoilers
in notifications and other places, clients are strongly encouraged to first upload the spoiler
Comment on lines +541 to +543
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The plain text (``body``) fallback for spoilers is a little different than the HTML-formatted
message as the ``body`` is often included as-is in notifications to the user. To prevent spoilers
in notifications and other places, clients are strongly encouraged to first upload the spoiler
When sending a spoiler, clients SHOULD provide the plain text fallback in ``body``.
The fallback SHOULD omit the spoiler text verbatim (but include the reason), since ``body``
contents can be used as-is in basic clients, notifications to the user etc. To prevent spoilers
showing up in such situations, clients are strongly encouraged to first upload the spoiler

Copy link
Member Author

Choose a reason for hiding this comment

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

The MSC does not make the fallback a SHOULD, anywhere (or I'm failing to see it). The MSC only ever references it as "the plaintext fallback" in a context of SHOULD upload the text, implying that it is required (in terms of language on the page).

To be clear, this concern is about the first SHOULD - the second SHOULD with respect to uploading the files is fine.

I'm happy to block this PR behind another MSC to clarify the intent, as this seems like it could be an opportunity for extended discussion which doesn't belong here.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree the original MSC is not perfectly clear on the modality of the fallback (whether it's SHOULD or MUST).

Copy link
Contributor

Choose a reason for hiding this comment

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

Soru intended to word it as a recommendation, however if it is ambiguous it seems like further discussion might be appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

#2557

text to the media/content repository, then use the returned content URI in a markdown-style link
as shown in the above examples. The markdown-style link's text should be ``Spoiler`` when no reason
is supplied and ``Spoiler for <reason>`` when a reason is supplied.

Clients SHOULD render spoilers differently with some sort of disclosure. For example, the client
could blur the actual text and ask the user to click on it for it to be revealed.

Server behaviour
----------------
Expand Down