Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Change Rich Quoting HTML Format over the wire #1701

Closed
wants to merge 4 commits into from

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Jan 13, 2018

To something a little richer that will look better on clients that do not yet support this feature whilst still keeping it as easy to parse

Fixes element-hq/element-web#5967
Fixes element-hq/element-web#5982

the quote target is sent in the cite attribute of the blockquote
to make parsing easier, it is also provided as a link within the body
for clients that do not yet understand rich quoting

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Copy link
Member Author

t3chguy commented Jan 13, 2018

TravisCI Chrome is still broken

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@dbkr
Copy link
Member

dbkr commented Jan 15, 2018

I think we'll have to do the due diligence here as this is basically spec stuff (even if the actual blockquote / cite is going into org.matrix.custom.html). My main concern here is that the reply metadata is only in the HTML body, so clients that don't render HTML now have to parse the HTML body to support rich quoting. It feels like we ought to have the quoting metadata outside of the body, but then this is starting to intersect with @ara4n 's message typing stuff too.

Shall we get some suggestions on a google doc and try to get some feedback?

@t3chguy
Copy link
Member Author

t3chguy commented Jan 16, 2018

So I'm all for instead throwing quotedEvent: ".../..." into the content of the m.room.message, it'd mean that there can only be one, but I consider that a good thing
so that an event could not be sent with 100s of the damn things

@dbkr
Copy link
Member

dbkr commented Jan 16, 2018

True, but in the context of a forum post, only being able to cite one message and in no particular point in your message feels quite limiting. I guess this is essentially the same argument as Pills where we eventually put everything into the HTML, so perhaps it's better to be consistent, although I still have the same concern there.

Anyway, arguing myself back & forth aside, unassigning myself to let others chime in.

@dbkr dbkr removed their assignment Jan 16, 2018
@t3chguy
Copy link
Member Author

t3chguy commented Jan 24, 2018

Because of element-hq/element-web#5967 (comment) the semantics are entirely different IMO.
Its more a linear reply to a single precursor. So a replyingToEvent field on m.room.message events makes most sense for this to me. Though would mean that "dumb" clients would simply render nothing.

@t3chguy
Copy link
Member Author

t3chguy commented Jan 29, 2018

The above would also fix (and seems to be the sanest/only sane way to fix) things like: element-hq/element-web#6061

@t3chguy
Copy link
Member Author

t3chguy commented Jan 29, 2018

Could also fix node replacing nastiness/race conditions such as element-hq/element-web#6059 (comment)

@t3chguy
Copy link
Member Author

t3chguy commented Feb 13, 2018

Replaced by #1741

@t3chguy t3chguy closed this Feb 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants