Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

QUnit.diff should be given dumped values before html escape #483

Closed
Krinkle opened this issue Oct 23, 2013 · 7 comments
Closed

QUnit.diff should be given dumped values before html escape #483

Krinkle opened this issue Oct 23, 2013 · 7 comments
Labels
Component: Diff Type: Bug Something isn't working right.

Comments

@Krinkle
Copy link
Member

Krinkle commented Oct 23, 2013

The parameters can be serialised by jsDump, but they shouldn't be escaped yet so that a diff library can diff the real values instead of something that is already harder to work with.

Afaik nothing relies on this, we should be able to move the escapeText call from around the values from jsDump to around the diff() call.

@Krinkle
Copy link
Member Author

Krinkle commented Oct 23, 2013

Use case: Started looking at jsdifflib for QUnit (thanks Ed Sanders), and that diff library outputs more sophisticated HTML (e.g. two-colum diff layout), but it takes text as input and ends up double escaped.

@jzaefferer
Copy link
Member

@Krinkle is this and the other issues labelled "diff" something you could look into?

@shivamdixit
Copy link
Contributor

@Krinkle If we move the escapeText call around the diff() call then the <del> and <ins> tags of diff are also escaped, because of which the output is all garbled.

@jzaefferer
Copy link
Member

@shivamdixit we should evaluate this issue in the context of using goodle-diff-patch-match (#772), since we're very likely moving to that diff implementation.

@shivamdixit
Copy link
Contributor

Yup, I evaluated this issue in the context of using google-diff-patch-match library only.

@jzaefferer
Copy link
Member

Okay. Not sure if we still need this issue, might be invalid in this context. I'll leave it open for now, maybe @Krinkle can chime in. Let's focus on other issues until then.

@jzaefferer
Copy link
Member

Closing this since we can't reproduce the issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Component: Diff Type: Bug Something isn't working right.
Development

No branches or pull requests

3 participants