data loss using line-spanning font tags on a bulleted list

[traumschule]

I mutated this ticket to the follow up bug as I was able to reproduce the "Note has no Title" message for a "lost note" in version 92cac7c (post 0.1alpha).

Steps to reproduce

• create a note with a bulleted list
• mark text that spans more than one bullet line
• select any font style
• quit tomboy, restart it and reopen the note
• the changes are not there

Observation

From a user's perspective damaged notes should be hidden from standard search, but clicking a checkbox could make them visible to give users a chance to copy content to a new note. (if there are no damaged notes, this checkbox should not appear of course).

"Note has no title"

This issue confuses me since #2, because when this note "disappeared" the file stayed and is since warned about on startup:

$ ./tomboy-ng
Note has no Title 3be61374-8545-4852-b5cc-11971e689c7f.note

This is the exact note content:

<?xml version="1.0" encoding="utf-8"?>
<note version="0.3" xmlns:link="http://beatniksoftware.com/tomboy/link" xmlns:size="http://beatniksoftware.com/tomboy/size" xmlns="http://beatniksoftware.com/tomboy">
  <title>17-11</title>
  <text xml:space="preserve"><note-content version="0.1">17-11
&lt;17-10 2017 17-12&gt;
<list><list-item dir="ltr">https://wiki.gnome.org/Apps/Tomboy</list-item></list>
<list><list-item dir="ltr">To<size:small>mboy next Gen http://www.bannons.id.au/tomboy</list-item></list>
</size:small>
http://www.bannons.id.au/tomboy/download-and-test
- bin 64bit http://www.bannons.id.au/uploads/downloads/Tomboy_NG
- Source http://www.bannons.id.au/uploads/downloads/Source.zip
- Lazarus https://sourceforge.net/projects/lazarus/files/Lazarus%20Linux%20i386%20DEB/Lazarus%201.8.0RC5/
- KControl http://www.bannons.id.au/tomboy.zip
https://github.com/tomboy-notes/tomboy/issues/66
</note-content></text>
  <last-change-date>2017-11-26T02:40:07.2070000+01:00</last-change-date>
  <last-metadata-change-date>2017-11-26T02:40:07.2070000+01:00</last-metadata-change-date>
  <create-date>2017-10-16T09:04:06.2312360+02:00</create-date>
  <cursor-position>1</cursor-position>
  <selection-bound-position>1</selection-bound-position>
  <width>1000</width>
  <height>626</height>
  <x>0</x>
  <y>0</y>
  <open-on-startup>False</open-on-startup>
</note>

From a first spot it looks correct, but I could narrow it down to the line

<list><list-item dir="ltr">To<size:small>mboy next Gen http://www.bannons.id.au/tomboy</list-item></list>
</size:small>

With a deeper look the <size:small> tag spans the </list-item> and breaks the note parser.
I don't know how but somehow I managed to break the list syntax by clicking on the 'small' button in the font submenu. Will try to reproduce it later.

[davidbannon]

Thats definitly bad xml, tags must not overlap. In fact the loadnote unit can probably cope with it, I made that very tolerent of such things but the tools that populate (eg) the SearchForm or are used in the syncing process use library xml readers. And they will not tolerate such things.

Now, interesting question, whats the date on that file ? If its pretty old, its possible you used an early release of tomboy-ng before I had that bullet worked out well. I sure had a lot of problems with it, due, mainly to the fact that KMemo notes a block of text bullet at the end of the para, not the start ??

Due to nature of this problem, messing up a note, I'd really like you to try and repeat it please.

[traumschule]

Interesting, so you made it clever enough to not save invalid xml (what happened in #2), downside is, changes are silently lost. The XML gets definitively messed up by font functions with marks spanning over <list-item> boundaries. As long it is not caught, data loss is inevitable.

[davidbannon]

Yes, my theory was that I had to be sure that any note that could get past the SearchForm layer had to be opened safely by LoadNote. The technology I used in loadnote is slower than the well tuned xml parsers that assume the xml will be ok and if not, just raise an exception.
When we are indexing a whole directory full of notes, or looking for a search term, has to be fast.
Question might be better asked is "what should we do with a note that fails first test ?" - at present we slip an warning out to console but usually nobody is watching. I don't think there is any point in poping up a warning box until we are ready to also offer the (inexperienced) user a means to fix it.
Another Question, "how did it happen" - realistically, bad xml should not happen. Yours may have happened as a result of using my dodgy early code. :-) I'd like to think it won't happen again but we cannot be sure.
Tomboy silently ignores too.

[traumschule]

All changes should be saved, no matter of the validity test.

I fear it's not only the early but current code that creates invalid XML by spanning <list:item> tags, but those never see disk at they are silently dropped, which is worse in my eyes.

Please try it:

• create a note with a bulleted list
• mark text that spans more than one bullet line
• select any font style
• quit tomboy, restart and reopen the note
• the changes are not there!

From a user's perspective damaged notes should be hidden from standard search, but clicking a checkbox could make them visible to give users a chance to copy content to a new note. (if there are no damaged notes, this checkbox should not appear of course).

[davidbannon]

OK, if thats the case, its a real problem because the bullet code is not pretty and took a lot of work !
To make matters worse, I'll be unavailable today and quite possibly tomorrow ....

[davidbannon]

OK, yep, it was a bad as I feared. I think that unit needs to be rewritten using library XML tools.
Anyway, it now passes my tests but given just how much I have had to change that code (134 insertions(+), 65 deletions(-) ) I think it really needs some more, aggressive tests.
The initial issue applied to any font change spanning either start or end of bullet list. There was also an issue of tag order where, if three or more attributes were applied, it was possible to mess up tag order.
Right now, I am unable to provoke a bud response but ....

[davidbannon]

For the record, I believe this issue was fixed with 6525c13 but instead of saying #21 I said #12.
My own tests and several days 'by the way' testing have revealed no more problems. But, as noted, it did involve an extensive rewrite of savenote.pas so I'm happy to leave it open until independent tests are also positive.

[traumschule]

There are still several issues with bullets.

1. Rightclick the tray icon and choose "New Note"
2. Insert
   1
   2
   3
3. Mark those lines and click Text -> Bullet
4. Mark line 2 and chose "Huge" => line three changes its font as well
5. Move the cursor to the beginning of line three and hit => line 2 looses its bullet (actually the three should have been added to line two)
6. Mark two lines, click "Highlight", quit tomboy-ng, restart and reopen the note. Here no text is highlighted (red)
   There are more like unbulletting does not seem to work reliably.
   This is with current HEAD (92cac7c).

[davidbannon]

I think you are confusing at least two issues here. I personally hate the way bullets are implemented right now but thats an issue that derives from KControls. In Kmemo, the bullet-ness of a bit of text is recorded in the paragraph marker associated with the end of the text, not the start. So, sneak up behind some bullet point and press backspace, it cancels the bullet. I already intercept backspace to some extent to remove the worst of the wierdness but doing that a lot more is necessary. Its on the todo list.
This ticket is about writing invalid xml from bullet points, thats a much, much more significant issue because it could potentially cause loss of data. I think we need treat "loss of data" as a very special and specific case. I think its fixed, at least I have not been able to trigger it again.

One thing that worries me, as an english speaker, there are no higher uncode characters in my test documents. I assume you do have such things in your notes ?

David

[traumschule]

i just tried again to highlight bulleted text, quit and found the change missing after a restart. can't you reproduce it?

[davidbannon]

If it saves, the highlight is certainly saved on my system. And it does it using good xml.

However, changing the highlight (and bold, italics etc) does not mark the note as dirty and therefore, unless you make some other change, the note is not saved again. Thats been the case from day one. You should be able to see that by watching the 'd' and 'c' top right of the note window. It becomes 'd' when the note is marked dirty, 'c' when saved. Highlight the bullet list, it stays as 'd'. Won't be saved if you exit. Make some other arbitrary change, it becomes 'd', will be saved after 10 seconds or on exit.

This ticket is about writing bad xml to disk, xml that prevents the user's note from being readable.

[traumschule]

Trying above steps with 760013f the note disappears and the console shows:

Note has no Title E0422F8F-1EBD-4050-AF3E-CAC605847671.note

[davidbannon]

Which steps do you refer ?
"There are still several issues ..." works perfectly for me -

Can you please look at the file E0422... and see what the problem is ? That might give me a hint about which action is triggering it off. I'm quite worried about the very different behaviour you see compared to me. As you see in the picture, Huge is where it should be. When I backspaced (is that what you mean by "=>") the following text moves up to previous line (I reversed that to do the highlight test).
Again. I am suspicious of unicode problems. I assume your keyboard puts non-askii character in automatically. The app must support that but I have so little experience in that space.
Don't suppose you know what scan code you get when you hit "=>" ?
David

[davidbannon]

OK, I can produce bad xml, not by editing a note but by hand crafting a note that is valid, it gets re-saved as invalid. I'm assuming this is the same bug as you see, if nothing else, because I have to start somewhere. It does save bad xml so that is a bad thing by definition.
I'm stuck with considering if I should keep working on my model or try and port to a library xml issue. If it was not for the bullet marker at end of text, either would be easy, sigh ....
Thanks for keeping on chasing this !
David

[davidbannon]

OK, finally, I am sure I have fixed this bug.