js-test-pre.js's escapeHTML should escape null characters so we can see them
Created attachment 63637 [details] Patch
Comment on attachment 63637 [details] Patch > - return text.replace(/&/g, "&").replace(/</g, "<"); > + return text.replace(/&/g, "&").replace(/</g, "<").replace(/\0/g, "\\0"); I was thinking we should chose a form that can be parsed such as � but perhaps there is no such thing. r=me
> I was thinking we should chose a form that can be parsed such as � but perhaps there is no such thing. The problem is that will get decoded by the HTML parser and turned into a null character again. :)
Comment on attachment 63637 [details] Patch Clearing flags on attachment: 63637 Committed r64796: <http://trac.webkit.org/changeset/64796>
All reviewed patches have been landed. Closing bug.
(In reply to comment #3) > The problem is that will get decoded by the HTML parser and turned into a null character again. :) Are you sure? I don’t think it will.
> Are you sure? I don’t think it will. http://trac.webkit.org/browser/trunk/WebCore/html/HTMLEntityParser.cpp#L66 Looks like the entity parser itself catches null and replaces it with U+FFFD.
The only downside with turning a null character into "\0" is that the characters "\0" also display as "\0". I guess I can’t think of any better thing to do, though, so I’m not sure what my point is!
> The only downside with turning a null character into "\0" is that the characters "\0" also display as "\0". That seems like a bug too, though. Isn't the output of these tests meant to look like a JavaScript string?
(In reply to comment #9) > That seems like a bug too, though. Isn't the output of these tests meant to look like a JavaScript string? On reflection, I erred in my original suggestion. The job of escapeHTML is to take a string and put it in a format where if it’s parsed as HTML you get the original string back. And it can almost do that but not quite, because of the special rule about null characters that makes it impossible in the general case. So I was wrong to suggest adding special handling of null characters to that function without renaming it. Tests should not need to serialize and then re-parse; fixing that would eliminate the need for escapeHTML entirely. Escaping for human readability is another type of function. I don't think the test machinery tries to do that at all, but it could. If there was a function that tried to do that, it should handle more control characters than just the null character, and possibly many other non-ASCII characters as well. That’s all from memory without studying js-test-pre.js further. I might have other ideas if I read it over.
Would you like me to revert this patch while we consider further?
IIRC, ggaren is the (or at least one of the) original author(s) of js-test-pre.js
There’s no need to revert the change. But we can come up with something better for the future. For this discussion it doesn’t matter who the original author of js-test-pre.js is. I’m pretty sure it was Maciej, though.