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

Deprecate the font HTML tag #1739

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Mar 5, 2024

Rationale:

MSC4077 allows to deprecate HTML tags that are deprecated in the WHATWG standard without requiring an MSC, if they can be replaced by tags with the same feature.

font is deprecated and can be replaced by span with the data-mx-bg-color and data-mx-color attributes.

So, HTML like this:

<font data-mx-bg-color="#FF0000" color="#00FF00">Some green text on a red background</font>

Can also be written like this:

<span data-mx-bg-color="#FF0000" data-mx-color="#00FF00">Some green text on a red background</span>

Preview: https://pr1739--matrix-spec-previews.netlify.app

Rationale:
MSC4077 allows to deprecate HTML tags
that are deprecated in the WHATWG standard,
if they can be replaced by tags with the same feature.

`font` is deprecated and can be replaced by `span`
with the `data-mx-bg-color` and `data-mx-color` attributes.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from a team as a code owner March 5, 2024 13:03
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
KitsuneRal
KitsuneRal previously approved these changes Mar 5, 2024
@tulir
Copy link
Member

tulir commented Mar 5, 2024

<font> is used more than <strike>. Strikethrough was always done using <del> in most clients, but the /rainbow command in the latest element web version outputs <font color="..."> tags. This also effectively reverts MSC2422

It might still be a good idea to remove it, but I think it'd be good to have a note saying it was previously allowed. Maybe mark it as deprecated somehow instead of removing entirely

@KitsuneRal
Copy link
Member

A note saying that clients should stop using font and switch to span would be nice indeed. As for <font color=…>, I surmise sticking to one way of colouring characters is a good thing.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh
Copy link
Contributor Author

zecakeh commented Mar 5, 2024

I added an info box for the deprecation.

tulir added a commit to maunium/matrix-react-sdk-old that referenced this pull request Mar 7, 2024
The font tag may be deprecated soon (matrix-org/matrix-spec#1739)

Signed-off-by: Tulir Asokan <tulir@maunium.net>
Copy link
Member

@tulir tulir left a comment

Choose a reason for hiding this comment

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

This looks fine now, but I'm not completely sure if it should require an MSC due to being used by many clients

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

LGTM. Given that the change in the clients is quite straightforward, I don't think we should hold back this change. After all, the whole HTML subset is recommended, not mandatory, and cases of similar minor non-compliance have happened and will happen. But I'd appreciate a third opinion from another client author, out of caution.

@turt2live turt2live self-requested a review March 12, 2024 03:12
@turt2live turt2live mentioned this pull request Mar 12, 2024
23 tasks
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I'm not a terribly active client dev myself these days, but the change seems reasonable given the context. Clients are asked to prefer span, but we aren't breaking anything.

Opening issues on a few clients would be appreciated, but not required. This might be an SCT responsibility to encourage the ecosystem to upgrade anyways.

@turt2live turt2live merged commit 4247cff into matrix-org:main Mar 19, 2024
12 checks passed
@zecakeh zecakeh deleted the deprecate-font branch March 19, 2024 22:01
github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this pull request Mar 19, 2024
* Use data-mx-color for rainbows

The font tag may be deprecated soon (matrix-org/matrix-spec#1739)

Signed-off-by: Tulir Asokan <tulir@maunium.net>

* Update tests

Signed-off-by: Tulir Asokan <tulir@maunium.net>

---------

Signed-off-by: Tulir Asokan <tulir@maunium.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants