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

IRC inverted colour scheme doesn't map to Matrix #62

Open
kegsay opened this issue Jun 2, 2016 · 10 comments
Open

IRC inverted colour scheme doesn't map to Matrix #62

kegsay opened this issue Jun 2, 2016 · 10 comments
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements.

Comments

@kegsay
Copy link
Member

kegsay commented Jun 2, 2016

https://matrix.org/jira/browse/BOTS-157

Notably background colours.

@kegsay kegsay added bug labels Jun 2, 2016
@kegsay kegsay self-assigned this Jul 14, 2016
@kegsay
Copy link
Member Author

kegsay commented Jul 18, 2016

Punting this due to other more pressing issues for the next release.

@kegsay kegsay removed their assignment Jul 18, 2016
@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Jul 27, 2016

Seems that vector is pretty strict when it comes to displaying anything other than <b> etc. and <font color="...">. It's likely that the client is filtering out all of the html that it doesn't like, so this could be an issue with any client that doesn't just blindly spam HTML that it believes to be safe.

So, this is an issue with vector or any client that filters out certain tags. There doesn't seem to be any documentation on the org.matrix.custom.html format. Matrix sanitizes html with this: https://github.com/matrix-org/matrix-react-sdk/blob/24223ae2b69debb33fa22fcda5aeba6fa93c93eb/src/HtmlUtils.js. The https://github.com/punkave/sanitize-html package is used, which doesn't actually provide a way of only allowing certain CSS properties. It could be modified slightly to allow this filtering.

@lukebarnard1
Copy link
Contributor

@dbkr suggests stripping the foreground colour when background colour is used. The reason being, when you have IRC asking for white text on black background (and say for the sake of argument a client has it's background set to white), if just the bg is stripped, the text isn't visible in the client. By stripping the foreground, you at least have legible text.

@lukebarnard1 lukebarnard1 removed their assignment Jul 28, 2016
@ara4n
Copy link
Member

ara4n commented Jul 28, 2016

stripping foreground colour would conversely risk black-on-black text
for intverted text for typical clients which have a dark-on-light
colourscheme.

Why wouldn't we just keep the colours intact?

On 28/07/2016 10:23, Luke Barnard wrote:

@dbkr https://github.com/dbkr suggests stripping the foreground
colour when background colour is used. The reason being, when you
have IRC asking for white text on black background (and say for the
sake of argument a client has it's background set to white), if just
the bg is stripped, the text isn't visible in the client. By
stripping the foreground, you at least have legible text.

— You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#62 (comment),

or mute the thread
https://github.com/notifications/unsubscribe-auth/ABO_vd_aqdaE4gZ5_hH8WZFzEzvWy60Qks5qaGb2gaJpZM4IsZV4.

@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Jul 28, 2016

An alternative is to create a custom tag transformation that takes
<font color="..." bgcolor="...">...</font>
and replaces it with
<font style="color:...;background-color:...">...</font>.

@ara4n
Copy link
Member

ara4n commented Jul 28, 2016

making up our own HTML attributes feels like a bad idea, and obviously we can't allow arbitrary CSS. I guess we could do something a bit dodgy like <font color="" data-matrix-bg-color="..."/> to specialcase it... :S

@kegsay
Copy link
Member Author

kegsay commented Sep 28, 2016

Unassigned; we have more pressing issues than this.

@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Mar 3, 2017

So, now that Riot supports data-mx[-bg]-color, this can be done!

@lukebarnard1 lukebarnard1 removed their assignment Mar 3, 2017
@joepie91
Copy link

Resurrecting this bug because it actually caused a knock-on bug, breaking auto-URLs in Riot (names redacted for privacy):

Selection_148

Producing this source, after bridging:

Selection_149

The original on IRC looked like this:

Selection_150

@Mikaela
Copy link
Contributor

Mikaela commented Apr 4, 2022

I think #1548 is somewhat related issue.

@justinbot justinbot added T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements. and removed bug labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements.
Projects
None yet
Development

No branches or pull requests

6 participants