[Libreoffice-commits] core.git: sw/qa sw/source writerfilter/source

Justin Luth (via logerrit) logerrit at kemper.freedesktop.org
Wed Apr 1 08:16:07 UTC 2020


 sw/qa/extras/ooxmlexport/data/tdf120852_readOnlyProtection.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport5.cxx                       |   22 ++++++-
 sw/source/filter/ww8/docxexport.cxx                             |   31 +++++++---
 writerfilter/source/dmapper/DomainMapper_Impl.cxx               |    2 
 writerfilter/source/dmapper/SettingsTable.cxx                   |   11 +++
 writerfilter/source/dmapper/SettingsTable.hxx                   |    1 
 6 files changed, 57 insertions(+), 10 deletions(-)

New commits:
commit 3af7be613526404276210a698f77e0187831b9b1
Author:     Justin Luth <justin.luth at collabora.com>
AuthorDate: Wed Mar 11 10:09:29 2020 +0300
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Wed Apr 1 10:15:29 2020 +0200

    tdf#120852 writerfilter: support read-only docProtection
    
    Setting the document to LoadReadonly nicely does not
    prompt the user to "press this button to edit".
    That is what we would generally want when
    Read-Only is enforced, so lets use that.
    The user can easily enter edit mode via the edit menu,
    if they want to temporarily override the protection,
    which seems natural and discoverable enough.
    
    There is a File menu - Properties - Security option
    that manages the LoadReadonly setting in LO. If the
    user turns that off, then export will also cancel
    enforcement of the readOnly grabbag item.
    
    The situation where read-only was not enforced before,
    but now is enforced by LO, is handled by _MarkAsFinal,
    so that case is ignored. In other words, there was no point
    in adding a WriterWantsToProtectReadOnly flag.
    See tdf#107690 for _MarkAsFinal fix.
    
    I had started going down the wrong patch of being
    innovative with boolean &=, not realizing
    that it now always evaluated the right side.
    Remove that bad example for other cut-and-pasters.
    In the end, this logic is much easier to understand
    anyway.
    
    Change-Id: Id26b283078a5fd62d662a26a96cfc461e0ba8459
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90323
    Tested-by: Jenkins
    Reviewed-by: Justin Luth <justin_luth at sil.org>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf120852_readOnlyProtection.docx b/sw/qa/extras/ooxmlexport/data/tdf120852_readOnlyProtection.docx
new file mode 100644
index 000000000000..02869aa5a74e
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf120852_readOnlyProtection.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
index a1b7ef6aad3b..511b8f0ff890 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
@@ -121,12 +121,30 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testfdo79008, "fdo79008.docx")
     /* File crashing while saving in LO.
      * Check if document.xml file is created after fix
      */
-     parseExport("word/document.xml");
+    parseExport("word/document.xml");
+
+    // Read-only is set, but it is not enforced, so it should be off...
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument *>(mxComponent.get());
+    CPPUNIT_ASSERT(pTextDoc);
+    CPPUNIT_ASSERT(!pTextDoc->GetDocShell()->IsSecurityOptOpenReadOnly());
 }
 
-DECLARE_OOXMLEXPORT_TEST(testTdf120852_readOnlyUnProtected, "tdf120852_readOnlyUnProtected.docx")
+DECLARE_OOXMLEXPORT_TEST(testTdf120852_readOnlyProtection, "tdf120852_readOnlyProtection.docx")
 {
+    if (xmlDocPtr pXmlSettings = parseExport("word/settings.xml"))
+    {
+        assertXPath(pXmlSettings, "/w:settings/w:documentProtection", "enforcement", "1");
+        assertXPath(pXmlSettings, "/w:settings/w:documentProtection", "edit", "readOnly");
+    }
 
+    // Read-only is set, so Enforcement must enable it.
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument *>(mxComponent.get());
+    CPPUNIT_ASSERT(pTextDoc);
+    CPPUNIT_ASSERT(pTextDoc->GetDocShell()->IsSecurityOptOpenReadOnly());
+}
+
+DECLARE_OOXMLEXPORT_TEST(testTdf120852_readOnlyUnProtected, "tdf120852_readOnlyUnProtected.docx")
+{
     // Readonly is not enforced, just a suggestion,
     // so when a section is protected, the document should enable forms protection.
     SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument *>(mxComponent.get());
diff --git a/sw/source/filter/ww8/docxexport.cxx b/sw/source/filter/ww8/docxexport.cxx
index ae63a3e13a2c..f9af8d2937cf 100644
--- a/sw/source/filter/ww8/docxexport.cxx
+++ b/sw/source/filter/ww8/docxexport.cxx
@@ -1083,12 +1083,13 @@ void DocxExport::WriteSettings()
     // Has themeFontLang information
     uno::Reference< beans::XPropertySet > xPropSet( m_pDoc->GetDocShell()->GetBaseModel(), uno::UNO_QUERY_THROW );
 
-    bool hasProtectionProperties = false;
+    bool bUseGrabBagProtection = false;
     bool bWriterWantsToProtect = false;
     bool bWriterWantsToProtectForm = false;
     bool bWriterWantsToProtectRedline = false;
     bool bHasRedlineProtectionKey = false;
     bool bHasDummyRedlineProtectionKey = false;
+    bool bReadOnlyStatusUnchanged = true;
     uno::Reference< beans::XPropertySetInfo > xPropSetInfo = xPropSet->getPropertySetInfo();
     if ( m_pDoc->getIDocumentSettingAccess().get(DocumentSettingId::PROTECT_FORM) ||
          m_pSections->DocumentIsProtected() )
@@ -1236,18 +1237,32 @@ void DocxExport::WriteSettings()
                             pAttributeList->add(FSNS(XML_w, nToken), sValue.toUtf8());
                             if ( nToken == XML_edit && sValue == "trackedChanges" )
                                 bIsProtectionTrackChanges = true;
+                            else if ( nToken == XML_edit && sValue == "readOnly" )
+                            {
+                                // Ignore the case where read-only was not enforced, but now is. That is handled by _MarkAsFinal
+                                bReadOnlyStatusUnchanged = m_pDoc->GetDocShell()->IsSecurityOptOpenReadOnly();
+                            }
                             else if ( nToken == XML_enforcement )
                                 bEnforced = sValue.toBoolean();
                         }
                     }
 
                     // we have document protection from input DOCX file
-                    // and in the case of change tracking protection, we didn't modify it
-                    hasProtectionProperties = !bIsProtectionTrackChanges || bHasDummyRedlineProtectionKey;
-                    // use grabbag if still valid/enforced
-                    // or leave as an un-enforced suggestion if Writer doesn't want to set any enforcement
-                    hasProtectionProperties &= bEnforced || !bWriterWantsToProtect;
-                    if ( hasProtectionProperties )
+                    if ( !bEnforced )
+                    {
+                        // Leave as an un-enforced suggestion if Writer doesn't want to set any enforcement
+                        bUseGrabBagProtection = !bWriterWantsToProtect;
+                    }
+                    else
+                    {
+                        // Check if the grabbag protection is still valid
+                        // In the case of change tracking protection, we didn't modify it
+                        // and in the case of read-only, we didn't modify it.
+                        bUseGrabBagProtection = (!bIsProtectionTrackChanges || bHasDummyRedlineProtectionKey)
+                                                && bReadOnlyStatusUnchanged;
+                    }
+
+                    if ( bUseGrabBagProtection )
                     {
                         sax_fastparser::XFastAttributeListRef xAttributeList(pAttributeList);
                         pFS->singleElementNS(XML_w, XML_documentProtection, xAttributeList);
@@ -1276,7 +1291,7 @@ void DocxExport::WriteSettings()
 
     WriteDocVars(pFS);
 
-    if (! hasProtectionProperties)
+    if ( !bUseGrabBagProtection )
     {
         // Protect form - highest priority
         // Section-specific write protection
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index fa6d76f13627..a5ba85300ea6 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -6640,6 +6640,8 @@ void DomainMapper_Impl::ApplySettingsTable()
             xSettings->setPropertyValue("AddParaTableSpacing", uno::makeAny(m_pSettingsTable->GetDoNotUseHTMLParagraphAutoSpacing()));
             if( m_pSettingsTable->GetProtectForm() )
                 xSettings->setPropertyValue("ProtectForm", uno::makeAny( true ));
+            if( m_pSettingsTable->GetReadOnly() )
+                xSettings->setPropertyValue("LoadReadonly", uno::makeAny( true ));
         }
         catch(const uno::Exception&)
         {
diff --git a/writerfilter/source/dmapper/SettingsTable.cxx b/writerfilter/source/dmapper/SettingsTable.cxx
index aed966abbecf..303de47c8f71 100644
--- a/writerfilter/source/dmapper/SettingsTable.cxx
+++ b/writerfilter/source/dmapper/SettingsTable.cxx
@@ -261,6 +261,7 @@ struct SettingsTable_Impl
     bool                m_bProtectForm;
     bool                m_bRedlineProtection;
     OUString            m_sRedlineProtectionKey;
+    bool                m_bReadOnly;
     bool                m_bDisplayBackgroundShape;
 
     uno::Sequence<beans::PropertyValue> m_pThemeFontLangProps;
@@ -298,6 +299,7 @@ struct SettingsTable_Impl
     , m_bProtectForm(false)
     , m_bRedlineProtection(false)
     , m_sRedlineProtectionKey()
+    , m_bReadOnly(false)
     , m_bDisplayBackgroundShape(false)
     , m_pThemeFontLangProps(3)
     , m_pCurrentCompatSetting(3)
@@ -368,6 +370,7 @@ void SettingsTable::lcl_attribute(Id nName, Value & val)
         // multiple DocProtect_edits should not exist. If they do, last one wins
         m_pImpl->m_bRedlineProtection = false;
         m_pImpl->m_bProtectForm = false;
+        m_pImpl->m_bReadOnly = false;
         switch (nIntValue)
         {
         case NS_ooxml::LN_Value_doc_ST_DocProtect_trackedChanges:
@@ -379,6 +382,9 @@ void SettingsTable::lcl_attribute(Id nName, Value & val)
         case NS_ooxml::LN_Value_doc_ST_DocProtect_forms:
             m_pImpl->m_bProtectForm = true;
             break;
+        case NS_ooxml::LN_Value_doc_ST_DocProtect_readOnly:
+            m_pImpl->m_bReadOnly = true;
+            break;
         }
         break;
     case NS_ooxml::LN_CT_DocProtect_enforcement: // 92039
@@ -661,6 +667,11 @@ bool SettingsTable::GetProtectForm() const
     return m_pImpl->m_bProtectForm && m_pImpl->m_DocumentProtection.m_bEnforcement;
 }
 
+bool SettingsTable::GetReadOnly() const
+{
+    return m_pImpl->m_bReadOnly && m_pImpl->m_DocumentProtection.m_bEnforcement;
+}
+
 bool SettingsTable::GetNoHyphenateCaps() const
 {
     return m_pImpl->m_bNoHyphenateCaps;
diff --git a/writerfilter/source/dmapper/SettingsTable.hxx b/writerfilter/source/dmapper/SettingsTable.hxx
index 3489cf0ac34b..e8cbe8abaf6e 100644
--- a/writerfilter/source/dmapper/SettingsTable.hxx
+++ b/writerfilter/source/dmapper/SettingsTable.hxx
@@ -75,6 +75,7 @@ class SettingsTable : public LoggedProperties, public LoggedTable
     bool GetDoNotExpandShiftReturn() const;
     bool GetNoColumnBalance() const;
     bool GetProtectForm() const;
+    bool GetReadOnly() const;
     bool GetLongerSpaceSequence() const;
     bool GetNoHyphenateCaps() const;
     sal_Int16 GetHypenationZone() const;


More information about the Libreoffice-commits mailing list