matrix-org / matrix-appservice-irc

Node.js IRC bridge for Matrix
Apache License 2.0
464 stars 150 forks source link

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

Open kegsay opened 8 years ago

kegsay commented 8 years ago

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

Notably background colours.

kegsay commented 8 years ago

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

lukebarnard1 commented 8 years ago

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 commented 8 years ago

@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.

ara4n commented 8 years ago

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 https://github.com/matrix-org/matrix-appservice-irc/issues/62#issuecomment-235831195,

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

lukebarnard1 commented 8 years ago

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 commented 8 years ago

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 commented 7 years ago

Unassigned; we have more pressing issues than this.

lukebarnard1 commented 7 years ago

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

joepie91 commented 5 years ago

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 commented 2 years ago

I think https://github.com/matrix-org/matrix-appservice-irc/issues/1548 is somewhat related issue.