Clean up OdtDocument cursor position tests

[peitschie]

Make OdtDocument cursor position tests more granular in how they are reported. This makes it easier to troubleshoot individual failing tests as the individual test is more specific.

• Extend core test runner to cope with test names containing angle brackets
• Verify the number of steps vs. expected number of cursor positions
• Remove doc length check for non-changing variables
• Report one test per cursor position document fragment

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2133/

[kossebau]

I propose to instead add a generic encodeForHTML (or better named) method which encodes <, > and & and to use it in all places where text is added to the HTML output.

[peitschie]

Hrm. Where else are you proposing this encode function being used?

I actually would like to never see such a thing in production code, as the somewhat uncontrolled string concatenation happening here seems like very bad practice to me.

[kossebau]

Hrm. Where else are you proposing this encode function being used?

Now that I tried to list them, there might be only that one other place in core.UnitTester which I saw initially, https://github.com/kogmbh/WebODF/blob/master/webodf/lib/core/UnitTester.js#L635 Somehow I expected more :)

This also made me look at BrowserRuntime.log where I learned that any log messages which start with < are set to innerHTML (thus need to be encoded) while all others are just passed into a new text node, so do not need encoding, okay. So still leaves the encoding problem with those inBrowser-special logging with custom spans, where the caller has to make sure things are properly encoded.
And I also learned from e.g. http://shebang.brandonmintern.com/foolproof-html-escaping-in-javascript/ that encoding is best left to the browser, e.g. by

function escapeHtml(str) {
    var div = document.createElement('div');
    div.appendChild(document.createTextNode(str));
    return div.innerHTML;
};

Hm. So what to do here without going over-board. Just the proposed fix still seems to be too hacky and done on case-by-case.

Oh well. Without a better idea, please just add a comment to it that this is an unclean fix to a certain problem.

And while talking, in line 637 it should be test.description(), not test.description, or? Please give that a patch as well.

[peitschie]

Brandon and I share the same opinion about the best way to do escape.

I don't mind leaving this as a nasty hack for the simple reason that if it looks like a bad hack, no-one will be tempted to copy it out of the file 🎌. Or... if they do, it should become extremely obvious to the reviewer that this code was never meant to make it into production!

But, I will happily comment the line to reinforce this!

[kossebau]

Still slightly confused why only < is cared for and not also > (and perhaps & for a possible test name char)? Because browsers can do proper error resolving with those other entities (no < before -> show < as char, no entity name after & -> show as char)?

[peitschie]

For most browser display purposes, escaping the open-tag is sufficient to ensure the code isn't parsed as HTML. Escaping the ampersand isn't necessary with the test names I'm using so far, but it would make sense for future-proofing.

However, as discussed on the ML, I want to throw away all this code and replace it with jasmine anyway. As a result, I don't feel it's a good investment to do any real future-proofing on this part of the code.

[kossebau]

Mh, okay. I would have made the escaping catch here a bit more broad given that examples show some non-[azAZ] chars to expect, but if you think this will soon be replaced and until then should just be hacked on case-by-case, well, there are more important things to discuss about :)
So please go with this and rebase and merge 🐹

[kossebau]

For the rest looks good to me and more useful indeed.

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2147/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2154/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2155/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2198/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2245/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2250/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2255/

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2266/

[peitschie]

Thanks for reviewing!