Merge lp:~mzanetti/reminders-app/fix-stale-qt-tags into lp:reminders-app

[Michael Zanetti]

fix data loss because of stale "-qt" tags in the markup

I caught a data loss bug that happened with a note of mine. Afaiu, this can happen if you create a note with reminders and just add some empty lines (paragraphs). The QTextEdit will set a style attribute to it like:

<p style="-qt-paragraph-type: empty;"> </p>

If this is sent to Evernote and the user edits this paragraph on the website, it will add content to the paragraph but still keep that attribute, not knowing about it but still trying to play nice on other clients and keep their special attributes.

When this comes back to reminders, QTextEdit will get confused about the paragraph being marked as an empty one but still having text, effectively, not displaying half of the content in this paragraph. Saving such a note will permanently drop parts of the paragraph and sync the result back to Evernote.

As a fix, I now completely clean up all the "-qt-*" tags generated by the QTextArea when converting the content back to ENML. That might make rendering next time a wee little slower, as the QTextEdit seems to regenerate those tags, but gives me a much better feeling than having stale attributes floating around - and fixes the content loss with my test note here.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:408
http://91.189.93.70:8080/job/reminders-app-ci/738/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2558
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2558/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/340
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-vivid-amd64-ci/160

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/738/rebuild

review: Approve (continuous-integration)

[Riccardo Padovani (rpadovani)]

I have got warnings compiling:

/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp: In member function ‘QString EnmlDocument::convert(const QString&, EnmlDocument::Type) const’:
/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp:137:54: warning: unknown escape sequence: '\-'
                                 style.remove(QRegExp("-qt-[a-z\-\: ]*;"));
                                                      ^
/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp:137:54: warning: unknown escape sequence: '\:'
/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp: In member function ‘void EnmlDocument::setRichText(const QString&)’:
/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp:370:54: warning: unknown escape sequence: '\-'
                                 style.remove(QRegExp("-qt-[a-z\-\: ]*;"));
                                                      ^
/home/rpadovani/Documents/ubuntu/touch/core-apps/reminders-app/fix-stale-qt-tags/src/libqtevernote/utils/enmldocument.cpp:370:54: warning: unknown escape sequence: '\:'

review: Needs Fixing

[lp:~mzanetti/reminders-app/fix-stale-qt-tags]

409. By Michael Zanetti on 2015-04-13

fix warnings

[Riccardo Padovani (rpadovani)]

LGTM

review: Approve

=== modified file 'src/libqtevernote/utils/enmldocument.cpp'
--- src/libqtevernote/utils/enmldocument.cpp	2015-03-08 21:00:25 +0000
+++ src/libqtevernote/utils/enmldocument.cpp	2015-04-13 20:45:34 +0000
@@ -128,15 +128,23 @@
                                 break;
                             }
                             if (type == TypeRichText) {
+                                QString style = attribute.value().toString();
+
+                                // Let's remove any "-qt" attributes that might have ended up in the ENML (earlier versions
+                                // of reminders, or other QTextEdit based clients). They are most likely outdated as
+                                // Evernote just ignores but still keeps them and might cause issues if the content inside
+                                // the <p> changed. TextArea will regenerate them anyways if it thinks they are useful.
+                                style.remove(QRegExp("-qt-[a-z-: ]*;"));
+
+                                // Now convert some of the ENML style tags to "-qt" tags in order to get the most out
+                                // of QTextEdit.
                                 if (attribute.value().contains("padding-left")) {
-                                    QString style = attribute.value().toString();
                                     int padding = style.split("padding-left:").at(1).split("px").first().toInt();
                                     int indent = padding / 30 * 4;
                                     style.replace(QRegExp("padding-left:[ 0-9]*px;"), "-qt-block-indent:" + QString::number(indent) + ";");
-                                    writer.writeAttribute("style", style);
-                                } else {
-                                    writer.writeAttribute(attribute);
                                 }
+
+                                writer.writeAttribute("style", style);
                             } else {
                                 writer.writeAttribute(attribute);
                             }
@@ -276,6 +284,8 @@
 
     writer.writeEndElement();
     writer.writeEndDocument();
+    qCDebug(dcEnml) << QString("************** Converting ENML to %1 **************").arg(type == TypeHtml ? "HTML" : "RichText");
+    qCDebug(dcEnml) << QString("Original EML document:") << m_enml;
     qCDebug(dcEnml) << QString("Converted to %1:").arg(type == TypeHtml ? "HTML" : "RichText") << html;
     return html;
 }
@@ -346,15 +356,20 @@
                     if (reader.name() == "p") {
                         foreach (const QXmlStreamAttribute &attribute, reader.attributes()) {
                             if (attribute.name() == "style") {
+                                QString style = attribute.value().toString();
+
+                                // First convert some of the "-qt" tags added by the QTextArea to ENML style tags
                                 if (attribute.value().contains("-qt-block-indent")) {
-                                    QString style = attribute.value().toString();
                                     int indent = style.split("-qt-block-indent:").at(1).split(";").first().toInt();
                                     int padding = indent / 4 * 30;
                                     style.replace(QRegExp("-qt-block-indent:[0-9]*;"), "padding-left:" + QString::number(padding) + "px;");
-                                    writer.writeAttribute("style", style);
-                                } else {
-                                    writer.writeAttribute(attribute);
                                 }
+
+                                // Now let's remove any left "-qt" attributes as they won't do any good to ENML
+                                // TextArea will regenerate them anyways when it loads a document without them.
+                                style.remove(QRegExp("-qt-[a-z-: ]*;"));
+
+                                writer.writeAttribute("style", style);
                             } else {
                                 writer.writeAttribute(attribute);
                             }
@@ -414,6 +429,11 @@
     }
 
     writer.writeEndDocument();
+
+    qCDebug(dcEnml) << QString("************** Converting RichText to ENML **************");
+    qCDebug(dcEnml) << QString("Original RichText:") << richText;
+    qCDebug(dcEnml) << QString("Converted to ENML:") << m_enml;
+
 }
 
 void EnmlDocument::markTodo(const QString &todoId, bool checked)