Fixes the check for the valid UTF8 symbols

[crsib]

Resolves: #2132
Resolves: #2134

(short description of the changes and the motivation to make the changes)

• I signed CLA
• The title of the pull request describes an issue it addresses
• If changes are extensive, then there is a sequence of easily reviewable commits
• Each commit's message describes its purpose and effects
• There are no behavior changes unnecessary for the stated purpose of the PR

Recommended:

• Each commit compiles and runs on my machine without known undesirable changes of behavior

[Paul-Licameli]

This looks harmless. I don't immediately understand why this fixes the bug. Is it because char is signed? But the signed-unsigned comparison would have coerced the negative to a large unsigned value.

More seriously I should ask why all the logic in XMLWriterx::XMLEsc was not reproduced in XMLUtf8BufferWriter::WriteEscaped. I should have asked that question with the earlier PR.

[crsib]

But the signed-unsigned comparison would have coerced the negative to a large unsigned value.

This is a signed-signed conversion, unfortunately.

More seriously I should ask why all the logic in XMLWriterx::XMLEsc was not reproduced in

The problem is that it was mostly reproduced, just with the exception of surrogate pairs handling.

[Paul-Licameli]

But the signed-unsigned comparison would have coerced the negative to a large unsigned value.

This is a signed-signed conversion, unfortunately.

DUH! I see 0x... and assume unsigned, but there wasn't ...u after it. Of course.

More seriously I should ask why all the logic in XMLWriterx::XMLEsc was not reproduced in

The problem is that it was mostly reproduced, just with the exception of surrogate pairs handling.

[Paul-Licameli]

But the signed-unsigned comparison would have coerced the negative to a large unsigned value.

This is a signed-signed conversion, unfortunately.

More seriously I should ask why all the logic in XMLWriterx::XMLEsc was not reproduced in

The problem is that it was mostly reproduced, just with the exception of surrogate pairs handling.

And we know the string has been converted to utf8 here

void XMLUtf8BufferWriter::WriteAttr(const std::string_view& name, const Identifier& value) 
{
   const wxScopedCharBuffer utf8Value = value.GET().utf8_str();

   WriteAttr(name, { utf8Value.data(), utf8Value.length() });
}

And that makes the surrogate handling unnecessary? It is ok to leave all the utf8 encodings un-escaped?

[crsib]

It is ok to leave all the utf8 encodings un-escaped

Surrogates are easier in UTF8. All the symbols we wanted to keep away from XML (although I do not really understand why) or escape are in the lower 7 bits. If the most significant bit is set - we definitely have a multiple bytes sequence, which we considered to be "safe" for XML.

And we know the string has been converted to utf8 here

This is an easy point for further improvements of the XMLWriter class though. The cases when we really need to convert the value from the wxString (or even to construct it!) are rare. At least the name argument should always be a std::string_view or even a const char*. This will like improve the serialization performance twice.