framework/source/accelerators/acceleratorconfiguration.cxx | 12 -
framework/source/accelerators/presethandler.cxx | 105 +++++++++++--
framework/source/inc/accelerators/presethandler.hxx | 27 +++
sw/qa/extras/odfexport/odfexport.cxx | 4
4 files changed, 125 insertions(+), 23 deletions(-)
New commits:
commit 79e63a2d07758097816ed09c20f21dd2dc450e2d
Author: David Hashe <m...@dhashe.com>
AuthorDate: Wed Apr 2 14:34:39 2025 -0400
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue May 27 13:51:44 2025 +0200
tdf#60700 stop creating empty Configurations2/accelerator in ODFs
This change stops the creation of empty Configurations2/accelerator
dirs on new documents that do not have anything that needs to be stored
there. Previously, LibreOffice would always create this directory.
Whenever an XStorage is opened readwrite on a path, it will immediately
create that path. LibreOffice was opening all of these paths readwrite.
So, the solution is to always open these paths as readonly, and then
when we need to write data to them we close them and re-open them as
readwrite. If we never need to write data to a path, then it won't be
created.
This patch specifically stops the creation of subdirectories that were
being created by DocumentAcceleratorConfiguration.
Other patches in the relation chain handle paths created by
UIConfigurationManager and ImageManager.
Test coverage was added in b9054fed3724a19479e221a8b4818a127a18aabe
$ CPPUNIT_TEST_NAME="Test testTdf60700_accelerators" make
CppunitTest_sw_odfexport
As far as I can tell, there is no way to actually attach custom
accelerators to a document at this time using the UI, so I did not do
any manual testing for this change.
(The closest thing to storing accelerators in the document is the work
from 65d2c9ca4d930bd9352e0b51f45e163cf4393241, but the macros don't
yet continue to work when you save the document in ODF)
With this change, a fresh document looks like this:
$ unzip -l Untitled\ 1.odt
Archive: Untitled 1.odt
Length Date Time Name
--------- ---------- ----- ----
39 2025-05-17 15:57 mimetype
0 2025-05-17 15:57 Configurations2/
12645 2025-05-17 15:57 styles.xml
899 2025-05-17 15:57 manifest.rdf
3563 2025-05-17 15:57 content.xml
866 2025-05-17 15:57 meta.xml
14745 2025-05-17 15:57 settings.xml
318 2025-05-17 15:57 Thumbnails/thumbnail.png
1061 2025-05-17 15:57 META-INF/manifest.xml
--------- -------
34136 9 files
Change-Id: I5e032d55e94114333ea7778afcc3df0fefe0a30f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185451
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/framework/source/accelerators/acceleratorconfiguration.cxx
b/framework/source/accelerators/acceleratorconfiguration.cxx
index 8876093fc4f6..1b3fc6cec9b2 100644
--- a/framework/source/accelerators/acceleratorconfiguration.cxx
+++ b/framework/source/accelerators/acceleratorconfiguration.cxx
@@ -310,17 +310,7 @@ sal_Bool SAL_CALL
XMLBasedAcceleratorConfiguration::isModified()
sal_Bool SAL_CALL XMLBasedAcceleratorConfiguration::isReadOnly()
{
- css::uno::Reference< css::io::XStream > xStream;
- {
- SolarMutexGuard g;
- xStream = m_aPresetHandler.openTarget(TARGET_CURRENT,
- css::embed::ElementModes::READWRITE); // open or create!
- }
-
- css::uno::Reference< css::io::XOutputStream > xOut;
- if (xStream.is())
- xOut = xStream->getOutputStream();
- return !(xOut.is());
+ return m_aPresetHandler.isReadOnly();
}
void SAL_CALL XMLBasedAcceleratorConfiguration::setStorage(const
css::uno::Reference< css::embed::XStorage >& /*xStorage*/)
diff --git a/framework/source/accelerators/presethandler.cxx
b/framework/source/accelerators/presethandler.cxx
index 3ab1b65abb87..e30a1b4683e3 100644
--- a/framework/source/accelerators/presethandler.cxx
+++ b/framework/source/accelerators/presethandler.cxx
@@ -76,19 +76,21 @@ TSharedStorages& SharedStorages()
PresetHandler::PresetHandler(css::uno::Reference< css::uno::XComponentContext
> xContext)
: m_xContext(std::move(xContext))
, m_eConfigType(E_GLOBAL)
+ , m_bShouldReopenRWOnStore(false)
{
}
PresetHandler::PresetHandler(const PresetHandler& rCopy)
{
- m_xContext = rCopy.m_xContext;
- m_eConfigType = rCopy.m_eConfigType;
- m_xWorkingStorageShare = rCopy.m_xWorkingStorageShare;
- m_xWorkingStorageNoLang = rCopy.m_xWorkingStorageNoLang;
- m_xWorkingStorageUser = rCopy.m_xWorkingStorageUser;
- m_lDocumentStorages = rCopy.m_lDocumentStorages;
- m_sRelPathShare = rCopy.m_sRelPathShare;
- m_sRelPathUser = rCopy.m_sRelPathUser;
+ m_xContext = rCopy.m_xContext;
+ m_eConfigType = rCopy.m_eConfigType;
+ m_xWorkingStorageShare = rCopy.m_xWorkingStorageShare;
+ m_xWorkingStorageNoLang = rCopy.m_xWorkingStorageNoLang;
+ m_xWorkingStorageUser = rCopy.m_xWorkingStorageUser;
+ m_lDocumentStorages = rCopy.m_lDocumentStorages;
+ m_sRelPathShare = rCopy.m_sRelPathShare;
+ m_sRelPathUser = rCopy.m_sRelPathUser;
+ m_bShouldReopenRWOnStore = rCopy.m_bShouldReopenRWOnStore;
}
PresetHandler::~PresetHandler()
@@ -347,8 +349,11 @@ void PresetHandler::connectToResource(
PresetHandler::EConfigType
// existing ones only!
// b) inside user layer we can (SOFT mode!) but sometimes we should not
(HARD mode!)
// create new empty structures. We should prefer using of any existing
structure.
- sal_Int32 eShareMode = (css::embed::ElementModes::READ |
css::embed::ElementModes::NOCREATE);
+ // c) in document mode we first open the storage as readonly and later
reopen it as
+ // readwrite if we need to store to it
+ sal_Int32 eShareMode = css::embed::ElementModes::READ;
sal_Int32 eUserMode = css::embed::ElementModes::READWRITE;
+ sal_Int32 eDocMode = css::embed::ElementModes::READ;
OUStringBuffer sRelPathBuf(1024);
OUString sRelPathShare;
@@ -381,14 +386,30 @@ void PresetHandler::connectToResource(
PresetHandler::EConfigType
// It has one layer only, and this one should be opened READ_WRITE.
// So we open the user layer here only and set the share layer
equals to it .-)
+ // We initially open the layer as readonly to avoid creating the
subdirectory
+ // for files that don't need it. We will reopen it as readwrite if
needed.
+
sRelPathBuf.append(sResource);
sRelPathUser = sRelPathBuf.makeStringAndClear();
sRelPathShare = sRelPathUser;
+ {
+ SolarMutexGuard g;
+
+ css::uno::Reference< css::beans::XPropertySet > xPropSet(
xDocumentRoot, css::uno::UNO_QUERY );
+ if ( xPropSet.is() )
+ {
+ tools::Long nOpenMode = 0;
+ if ( xPropSet->getPropertyValue(u"OpenMode"_ustr) >>=
nOpenMode )
+ m_bShouldReopenRWOnStore = nOpenMode &
css::embed::ElementModes::WRITE;
+ }
+ }
+
try
{
- xUser = m_lDocumentStorages.openPath(sRelPathUser , eUserMode
);
+ xUser = m_lDocumentStorages.openPath(sRelPathUser , eDocMode
);
xShare = xUser;
+
}
catch(const css::uno::RuntimeException&)
{ throw; }
@@ -449,9 +470,12 @@ void PresetHandler::copyPresetToTarget(std::u16string_view
sPreset,
// don't check our preset list, if element exists
// We try to open it and forward all errors to the user!
+ maybeReopenStorageAsReadWrite();
+
css::uno::Reference< css::embed::XStorage > xWorkingShare;
css::uno::Reference< css::embed::XStorage > xWorkingNoLang;
css::uno::Reference< css::embed::XStorage > xWorkingUser;
+
{
SolarMutexGuard g;
xWorkingShare = m_xWorkingStorageShare;
@@ -506,6 +530,10 @@ css::uno::Reference< css::io::XStream >
PresetHandler::openPreset(std::u16string
css::uno::Reference< css::io::XStream > PresetHandler::openTarget(
std::u16string_view sTarget, sal_Int32 const nMode)
{
+ if (nMode & css::embed::ElementModes::WRITE) {
+ maybeReopenStorageAsReadWrite();
+ }
+
css::uno::Reference< css::embed::XStorage > xFolder;
{
SolarMutexGuard g;
@@ -619,6 +647,63 @@ void
PresetHandler::removeStorageListener(XMLBasedAcceleratorConfiguration* pLis
}
}
+bool PresetHandler::isReadOnly() {
+ SolarMutexGuard g;
+
+ if (m_eConfigType == E_DOCUMENT)
+ {
+ if ( css::uno::Reference<css::embed::XStorage> xDocumentRoot =
m_lDocumentStorages.getRootStorage(); xDocumentRoot.is() )
+ {
+ css::uno::Reference< css::beans::XPropertySet > xPropSet(
xDocumentRoot, css::uno::UNO_QUERY );
+ if ( xPropSet.is() )
+ {
+ tools::Long nOpenMode = 0;
+ if ( xPropSet->getPropertyValue(u"OpenMode"_ustr) >>=
nOpenMode )
+ return !( nOpenMode & css::embed::ElementModes::WRITE );
+ }
+ }
+ }
+ else
+ {
+ if ( m_xWorkingStorageUser.is() )
+ {
+ css::uno::Reference< css::beans::XPropertySet > xPropSet(
m_xWorkingStorageUser, css::uno::UNO_QUERY );
+ if ( xPropSet.is() )
+ {
+ tools::Long nOpenMode = 0;
+ if ( xPropSet->getPropertyValue(u"OpenMode"_ustr) >>=
nOpenMode )
+ return !( nOpenMode & css::embed::ElementModes::WRITE );
+ }
+ }
+ }
+
+ return true;
+}
+
+void PresetHandler::maybeReopenStorageAsReadWrite() {
+ SolarMutexGuard g;
+
+ if ((m_eConfigType == E_DOCUMENT) && m_bShouldReopenRWOnStore)
+ {
+ m_bShouldReopenRWOnStore = false;
+ try
+ {
+ forgetCachedStorages();
+
+ sal_Int32 eUserMode = css::embed::ElementModes::READWRITE;
+ m_xWorkingStorageUser = m_lDocumentStorages.openPath(
m_sRelPathUser, eUserMode );
+ m_xWorkingStorageShare = m_xWorkingStorageUser;
+ m_xWorkingStorageNoLang = m_xWorkingStorageUser;
+
+ }
+ catch(const css::uno::RuntimeException&)
+ { throw; }
+ catch(const css::uno::Exception&)
+ { forgetCachedStorages(); }
+ }
+}
+
+
// static
css::uno::Reference< css::embed::XStorage >
PresetHandler::impl_openPathIgnoringErrors(const OUString& sPath ,
sal_Int32 eMode ,
diff --git a/framework/source/inc/accelerators/presethandler.hxx
b/framework/source/inc/accelerators/presethandler.hxx
index 18a20a4b2016..0800c8c8353d 100644
--- a/framework/source/inc/accelerators/presethandler.hxx
+++ b/framework/source/inc/accelerators/presethandler.hxx
@@ -112,6 +112,10 @@ class PresetHandler
OUString m_sRelPathShare;
OUString m_sRelPathUser;
+ /** @short in document mode, we open the storage as readonly at first,
+ then reopen it as readwrite the first time we store. */
+ bool m_bShouldReopenRWOnStore;
+
// native interface
public:
@@ -280,10 +284,33 @@ class PresetHandler
void addStorageListener(XMLBasedAcceleratorConfiguration* pListener);
void removeStorageListener(XMLBasedAcceleratorConfiguration*
pListener);
+
+ /** @short check if the connected working user storage is readonly
+
+ @descr In document mode, we check whether the document root is
readonly.
+ The document root is the parent of the working user
storage.
+ We always initially open the working user storage as
readonly in
+ document mode, but we may re-open it as readwrite later if
we
+ need to. In other modes, we just directly check whether
the working
+ user storage is readonly.
+
+ @return true if the connected working user storage is readonly,
false if not
+ */
+ bool isReadOnly();
+
// helper
private:
+ /** @short maybe reopen the working user storage as readwrite in
document mode
+
+ @descr In document mode, we always initially open the working
user storage
+ as readonly, and we remember whether we want to reopen it
as readwrite
+ on the first store. This reopens the working user storage
as readwrite
+ if we are in document mode, we want to, and we haven't yet.
+ */
+ void maybeReopenStorageAsReadWrite();
+
/** @short open a config path ignoring errors (catching exceptions).
@descr We catch only normal exceptions here - no runtime
exceptions.
diff --git a/sw/qa/extras/odfexport/odfexport.cxx
b/sw/qa/extras/odfexport/odfexport.cxx
index ce18c0c0fa84..9716a974b764 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -1554,8 +1554,8 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf60700_directories)
}
}
- // There should be one element ("accelerator") within Configurations2/ on
a fresh document.
- CPPUNIT_ASSERT_EQUAL(1, nMatches);
+ // There should be zero elements within Configurations2/ on a fresh
document.
+ CPPUNIT_ASSERT_EQUAL(0, nMatches);
}
