Properly deal with </font>

[Jojo-Schmitz]

instead of ignoring it altogether, reset font face and -size back to what it was before <font ...>

To allow stuff like this in instrument names:

Sopran
<font size="8">(u. Gemeinde)</font>
Alt

Which currently results in the last part of the string ("Alt") to also be in size (8) of the middle part instead of having the same size as the 1st part ("Sopran"):

Now looks like this>

Also useful for header/footer texts, which also can't get formatted with different parts

[cbjeukendrup]

I suddenly wondered: can <font></font> tags be nested? If so, then just some String/double variables won't be enough, and instead we'd need to do something with a std::stack.

[Jojo-Schmitz]

Well, yes, theoretically they could, but as in the use case I have in mind they'd be manually added, we might neglect that problem?

[Jojo-Schmitz]

As far as I can tell MuseScore itself never writes </font> tags

[cbjeukendrup]

Ah, yes, I see. MuseScore rather writes <font/> tags. I guess if you want to do it really strictly, then you'd need to set prevFontFace/…Size only for <font> tags and not for <font/>. But it doesn't make much of a difference.

I wonder a little bit though whether it makes sense at all to try to support reading constructions that MuseScore itself never writes. But okay, this change won't do any harm probably, and it's quite logical. Maybe we should someday adjust the writing code to this :)

[Jojo-Schmitz]

Added protection from storing prev font size and face when not using an end tag

[cbjeukendrup]

For QA: this PR just needs to be tested for regressions in saving-and-reloading and copy-pasting of formatted text, since the added functionality is not visible to users

[Jojo-Schmitz]

The last commits ain't really mandatory... More a 1st step towards that Maybe we should someday adjust the writing code to this

[Jojo-Schmitz]

The unit test failure of my latest changes (2nd to last commit) reveal that this code is also used for MusicXML im-/export.
Question is whather a) that matters and b) how to fix, Is maybe MScore more correct than MScore Text and just that one tests needs to get adjusted?

Sorry, @cbjeukendrup, guess I'd need another review

[Jojo-Schmitz]

Ah, I know now what causes this: the space in the font face name is causing the string separation to fail...
But how else to parse the string?

[cbjeukendrup]

Maybe something in the direction of:

    } else if (token.startsWith(u"font ")) {
         String remainder = token.mid(5); // maybe add `.trimmed() here just to be safe?
         for (; !remainder.empty();) {
             if (remainder.startsWith(u"size=\"")) {
                 // the value of the `size` attribute starts at index 6
                 // find the first index of a double quote _after_ index 6
                 size_t endQuoteIndex = remainder.indexOf(u'"', 6);
                 double fontSize = remainder.mid(6, endQuoteIndex - 6).toDouble();
                 // now do things with fontSize
                 // and adjust `remainder` for the next attribute
                 remainder = remainder.mid(endQuoteIndex + 1).trimmed(); // remove whitespace from beginning/end
             } else if (remainder.startsWith(u"face=\"")) {
                 // basically the same but without `toDouble()`
             } else {
                 LOGD("cannot parse html property <%s> in text <%s>", muPrintable(s), muPrintable(m_text));
                 return false;
             }
         }
         return true;
     } else if (token == u"/font") {

Of course those lengthy messy comments are only for illustration :)
And it's probably full of off-by-one errors, but this time of the day is not the best moment for thinking carefully about that 😄 but you doubtlessly get the idea.

[Jojo-Schmitz]

OK, I believe this PR is OK now

[DmitryArefiev]

@Jojo-Schmitz @cbjeukendrup Tested on Win10.

It doesn't save font size settings (after copy/paste text) after reopening the score.
But if to open in master and save the score, it opens fine in PR's build

bandicam.2024-04-05.18-44-09-052.mp4

[DmitryArefiev]

Also, there is an issue when formatting text in Staff/Part properties dialog>Text frame, but the same in master (will log it separately)

bandicam.2024-04-05.18-50-21-864.mp4

[Jojo-Schmitz]

Not really the purpose of the PR. It is to allow partial formating strings that can't be partially formatted using the UI, like instrument names, and header and footer texts.
For staff texts and frame texts other means are available

[Jojo-Schmitz]

The lost linefeed on save/reload is indeed odd.

[cbjeukendrup]

Also, there is an issue when formatting text in Staff/Part properties dialog>Text frame, but the same in master (will log it separately)

I think that already exists as #19189

[cbjeukendrup]

Not really the purpose of the PR. It is to allow partial formating strings that can't be partially formatted using the UI, like instrument names, and header and footer texts. For staff texts and frame texts other means are available

@Jojo-Schmitz Fact is, that saving/reloading fails for texts where the font face or size of (part of) the text is changed, which is a regression on master.

[cbjeukendrup]

This code seems to get hit with the / from <font />, maybe that is related to the problem

[Jojo-Schmitz]

OK, let's break earlier on that case

That (alone) doesn't help though.
I'm at a loss to find where and why it fails

[Jojo-Schmitz]

Rebased to check whether #22475 makes a difference

[Jojo-Schmitz]

Hmm, why is this engraving test failing?

9: --- /home/runner/work/MuseScore/MuseScore/src/engraving/tests/expression_data/expression-1-ref.mscx	2024-04-20 09:36:41.516683602 +0000
9: +++ expression-1.mscx	2024-04-20 09:59:49.129177961 +0000
9: @@ -146,6 +146,7 @@
9:            <Expression>
9:              <placement>above</placement>
9:              <snapToDynamics>0</snapToDynamics>
9: +            <italic>0</italic>
9:              <offset x="0" y="-4.72943"/>
9:              <text><i>expr </i><u>text</u></text>
9:              </Expression>
9:    <diff -u /home/runner/work/MuseScore/MuseScore/src/engraving/tests/expression_data/expression-1-ref.mscx expression-1.mscx failed, code: 1 

As the italic is explicity set by the <i>...</i> tags inside the string, for just a part of it, I thing setting it to 0 (AKA false) for the entire string there isn't wrong, or is it, just seems superfluous?
I think it does (IMHO rightfully!) result in the "Reset to default" button to get activated on this particular string.
Edit: no it does not... it just toggles the Italic button in the string's properties to off.
It does match what the source mscx does though, it too has that line the unit test asks to get added to the ref file.
So fixing the one ref. file might be the way to go?

Quite a few musicxml tests fail too, seems to be a very similar issue, adding a tag that doesn't really seem needed, but isn't wrong either:

14: +++ testWedgeOffset.mscx	2024-04-20 11:07:30.792268251 +0000
14: @@ -122,6 +122,7 @@
14:            </Text>
14:          <Text>
14:            <style>subtitle</style>
14: +          <size>16</size>
14:            <offset x="0" y="9.92425"/>
14:            <text><font size="16"/>MuseScore Testcase</text>
14:            </Text>
14:    <diff -u /home/runner/work/MuseScore/MuseScore/src/importexport/musicxml/tests/data/testWedgeOffset_ref.mscx testWedgeOffset.mscx 

But only for subtitle, not for title?!?

Anyway, none these failed prior to the rebase as far as I remeber, what other changes might have caused this?

It doesn't save font size settings

But maybe it has fixed this?