ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#71261: Allow spaces in writer table names

Browse Source 
Drop the space character from the places that rejected it in
table names:

- Insert Table dialog: text filter on the name entry
- Table Properties dialog: validation in DeactivatePage that
  showed an info dialog
- Navigator rename: forbidden characters list
- UNO SwXTextTable::setName: check that threw RuntimeException

Other illegal characters (".<>") are still blocked, since they
are incorrectly handled in formulas and chart data sources.

Also remove the now unused STR_WRONG_TABLENAME string.

Allowing spaces exposed that a chart linked to such a table lost
its data connection after save/reload, so fix the table name
(de)serialization on both sides of the link:

- ODF export (xmltexte.cxx) wrote draw:notify-on-update-of-ranges
  wrapped in single quotes when the name contained a space; write
  it unmodified instead (the attribute also allows a plain string,
  not only a cellRangeAddressList).
- ODF import (xmltexti.cxx) only unquotes the legacy single-quoted
  form when the value actually starts with a quote, so both old and
  new documents are read correctly.
- SwChartDataProvider::convertRangeFromXML split the space separated
  range list with a naive tokenizer that broke inside a single-quoted
  table name; split it in a quote-aware way so the chart range still
  resolves to the table (instead of falling back to the chart's
  internal local-table).

Added regression tests:
- verify that the Insert Table dialog accepts spaces in the
  table name
- verify that a table name with a space can be set via UNO
  and is preserved
- verify that the Table Properties dialog accepts and saves
  a table name containing a space
- verify that, after save/reload, the chart's
  notify-on-update-of-ranges keeps the unquoted table name
- verify the convertRangeToXML/convertRangeFromXML round-trip for
  a table name containing a space

Fix found with the help of Claude Opus.

Change-Id: I92bbed3bfed5c90af4242c396608dd3802cd126a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/204381
Tested-by: Jenkins
Reviewed-by: Heiko Tietze <heiko.tietze@documentfoundation.org>
This commit is contained in:
Paolo Benvenuto
2026-08-20 10:04:58 +02:00
committed by Heiko Tietze
parent 82f7b95f64
commit d3b0de73da
11 changed files with 143 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/inc/strings.hrc
-1
View File
@@ -342,7 +342,6 @@
#define STR_CAPTION_FRAME NC_("STR_CAPTION_FRAME", "%PRODUCTNAME Writer Frame")
#define STR_CAPTION_GRAPHIC NC_("STR_CAPTION_GRAPHIC", "%PRODUCTNAME Writer Image")
#define STR_CAPTION_OLE NC_("STR_CAPTION_OLE", "Other OLE Objects")
#define STR_WRONG_TABLENAME NC_("STR_WRONG_TABLENAME", "The name of the table must not contain spaces.")
#define STR_ERR_TABLE_MERGE NC_("STR_ERR_TABLE_MERGE", "Selected table cells are too complex to merge.")
#define STR_SRTERR NC_("STR_SRTERR", "Cannot sort selection")
sw/qa/core/doc/doc.cxx
+13
View File
@@ -745,6 +745,19 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testTableNameCollisionUpdatesChart)
assertXPath(pXmlDoc, "//draw:object", "notify-on-update-of-ranges", u"Table3");
}
CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testTableNameWithSpaceUpdatesChart)
{
createSwDoc("tdf171549.fodt");
SwFrameFormat* pFrameFormat = getSwDoc()->FindTableFormatByName(UIName("Table1"));
getSwDocShell()->GetEditShell()->SetTableName(*pFrameFormat, UIName("Renamed Table"));
saveAndReload(TestFilter::ODT);
xmlDocUniquePtr pXmlDoc = parseExport(u"content.xml"_ustr);
assertXPath(pXmlDoc, "//draw:object", "notify-on-update-of-ranges", u"Renamed Table");
}
CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testTableNameUpdatePropagatedToFormulas)
{
createSwDoc("tdf83196.fodt");
sw/qa/python/check_table.py
+12
View File
@@ -615,6 +615,18 @@ class CheckTable(unittest.TestCase):
xCellRangeString = xChartDataProvider.convertRangeFromXML("Table1.$A$1:.$C$3")
self.assertEqual("Table1.A1:C3", xCellRangeString)
# tdf#71261: a table name containing spaces must round-trip through the
# chart XML range conversions. In the XML form such a name is wrapped
# in single quotes, so the (space separated) range list parsing must
# not break inside the quoted name.
xTable.Name = "Table with space"
xml = xChartDataProvider.convertRangeToXML('Table with space.A1:C3')
self.assertEqual("'Table with space'.$A$1:.$C$3", xml)
xCellRangeString = xChartDataProvider.convertRangeFromXML("'Table with space'.$A$1:.$C$3")
self.assertEqual("Table with space.A1:C3", xCellRangeString)
xDoc.dispose()
def test_splitRangeHorizontal(self):
sw/qa/uitest/table/tdf71261.py
+59
View File
@@ -0,0 +1,59 @@
# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*-
#
# This file is part of the LibreOffice project.
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
from uitest.framework import UITestCase
from uitest.uihelper.common import get_state_as_dict, select_pos
from libreoffice.uno.propertyvalue import mkPropertyValues
class tdf71261(UITestCase):
def test_insert_table_name_with_space(self):
with self.ui_test.create_doc_in_start_center("writer") as document:
with self.ui_test.execute_dialog_through_command(".uno:InsertTable") as xDialog:
xNameEdit = xDialog.getChild("nameedit")
xNameEdit.executeAction("TYPE", mkPropertyValues({"KEYCODE": "CTRL+A"}))
xNameEdit.executeAction("TYPE", mkPropertyValues({"TEXT": "Table 1"}))
self.assertEqual(get_state_as_dict(xNameEdit)["Text"], "Table 1")
tables = document.getTextTables()
self.assertEqual(tables[0].getName(), "Table 1")
def test_uno_set_table_name_with_space(self):
with self.ui_test.create_doc_in_start_center("writer") as document:
with self.ui_test.execute_dialog_through_command(".uno:InsertTable") as xDialog:
pass # create a table with the default name
tables = document.getTextTables()
tables[0].setName("My Table")
self.assertEqual(tables[0].getName(), "My Table")
def test_table_properties_name_with_space(self):
with self.ui_test.create_doc_in_start_center("writer") as document:
with self.ui_test.execute_dialog_through_command(".uno:InsertTable") as xDialog:
pass # create a table with the default name
with self.ui_test.execute_dialog_through_command(".uno:TableDialog") as xDialog:
tabcontrol = xDialog.getChild("tabcontrol")
select_pos(tabcontrol, "0")
name = xDialog.getChild("name")
name.executeAction("TYPE", mkPropertyValues({"KEYCODE": "CTRL+A"}))
name.executeAction("TYPE", mkPropertyValues({"TEXT": "My Table"}))
with self.ui_test.execute_dialog_through_command(".uno:TableDialog") as xDialog:
tabcontrol = xDialog.getChild("tabcontrol")
select_pos(tabcontrol, "0")
name = xDialog.getChild("name")
self.assertEqual(get_state_as_dict(name)["Text"], "My Table")
tables = document.getTextTables()
self.assertEqual(tables[0].getName(), "My Table")
# vim: set shiftwidth=4 softtabstop=4 expandtab:
sw/source/core/unocore/unochart.cxx
+35 -6
View File
@@ -1684,6 +1684,33 @@ OUString SAL_CALL SwChartDataProvider::convertRangeToXML( const OUString& rRange
return aRes.makeStringAndClear();
}
// Split an XML cell-range-address list into its space-separated sub-ranges,
// keeping spaces that occur inside a single-quoted table name (e.g.
// "'Table 1'.$A$1") together. A backslash escapes the following character.
static std::vector<OUString> lcl_SplitXMLRanges( const OUString& rXMLRange )
{
std::vector<OUString> aRanges;
bool bInQuotation = false;
sal_Int32 nStart = 0;
for (sal_Int32 i = 0; i < rXMLRange.getLength(); ++i)
{
sal_Unicode c = rXMLRange[i];
if (c == '\\') // skip the escaped character
++i;
else if (c == '\'')
bInQuotation = !bInQuotation;
else if (c == ' ' && !bInQuotation)
{
if (i > nStart)
aRanges.push_back(rXMLRange.copy(nStart, i - nStart));
nStart = i + 1;
}
}
if (nStart < rXMLRange.getLength())
aRanges.push_back(rXMLRange.copy(nStart));
return aRanges;
}
OUString SAL_CALL SwChartDataProvider::convertRangeFromXML( const OUString& rXMLRange )
{
SolarMutexGuard aGuard;
@@ -1697,19 +1724,22 @@ OUString SAL_CALL SwChartDataProvider::convertRangeFromXML( const OUString& rXML
// multiple ranges are delimited by a ' ' like in
// "Table1.$A$1:.$A$4 Table1.$C$2:.$C$5" the same table must be used in all ranges!
// A table name containing spaces is single-quoted, so the split must not
// break inside such a quoted name.
OUString aFirstFoundTable; // to check that only one table will be used
sal_Int32 nPos = 0;
do
bool bFirstTableSet = false;
for (const OUString& aRange : lcl_SplitXMLRanges(rXMLRange))
{
OUString aRange( rXMLRange.getToken(0, ' ', nPos) );
//!! following objects and function are implemented in XMLRangeHelper.?xx
//!! which is a copy of the respective file from chart2 !!
XMLRangeHelper::CellRange aCellRange( XMLRangeHelper::getCellRangeFromXMLString( aRange ));
// check that there is only one table used in all ranges
if (aFirstFoundTable.isEmpty())
if (!bFirstTableSet)
{
aFirstFoundTable = aCellRange.aTableName;
bFirstTableSet = true;
}
if (aCellRange.aTableName != aFirstFoundTable)
throw lang::IllegalArgumentException();
@@ -1727,7 +1757,6 @@ OUString SAL_CALL SwChartDataProvider::convertRangeFromXML( const OUString& rXML
aRes.append(";");
aRes.append(aTmp);
}
while (nPos>0);
return aRes.makeStringAndClear();
}
sw/source/core/unocore/unotbl.cxx
+1 -2
View File
@@ -3010,8 +3010,7 @@ void SwXTextTable::setName(const OUString& rName)
SwFrameFormat* pFormat = GetFrameFormat();
if ((!pFormat && !m_pImpl->IsDescriptor()) ||
rName.isEmpty() ||
rName.indexOf('.')>=0 ||
rName.indexOf(' ')>=0 )
rName.indexOf('.')>=0 )
throw uno::RuntimeException();
if(pFormat)
sw/source/filter/xml/xmltexte.cxx
+4 -30
View File
@@ -317,37 +317,11 @@ void SwXMLTextParagraphExport::_exportTextEmbedded(
}
if( SV_EMBEDDED_OWN == nType && !pOLENd->GetChartTableName().isEmpty() )
{
OUString sRange( pOLENd->GetChartTableName().toString() );
OUStringBuffer aBuffer( sRange.getLength() + 2 );
for( sal_Int32 i=0; i < sRange.getLength(); i++ )
{
sal_Unicode c = sRange[i];
switch( c )
{
case ' ':
case '.':
case '\'':
case '\\':
if( aBuffer.isEmpty() )
{
aBuffer.append( OUString::Concat("\'") + sRange.subView(0, i) );
}
if( '\'' == c || '\\' == c )
aBuffer.append( '\\' );
[[fallthrough]];
default:
if( !aBuffer.isEmpty() )
aBuffer.append( c );
}
}
if( !aBuffer.isEmpty() )
{
aBuffer.append( '\'' );
sRange = aBuffer.makeStringAndClear();
}
// Export the table name as-is; XML attribute quoting handles any
// spaces or special characters. Older versions wrapped names with
// spaces in single quotes — the importer still accepts that form.
rXMLExport.AddAttribute( XML_NAMESPACE_DRAW, XML_NOTIFY_ON_UPDATE_OF_RANGES,
sRange );
pOLENd->GetChartTableName().toString() );
}
eElementName = SV_EMBEDDED_OUTPLACE==nType ? XML_OBJECT_OLE
: XML_OBJECT;
sw/source/filter/xml/xmltexti.cxx
+16 -53
View File
@@ -344,73 +344,36 @@ uno::Reference< XPropertySet > SwXMLTextImportHelper::createAndInsertOLEObject(
SwOLENode *pOLENode = pNdIdx->GetNodes()[pNdIdx->GetIndex() + 1]->GetOLENode();
OSL_ENSURE( pOLENode, "Where is the OLE node" );
OUStringBuffer aBuffer( rTableName.getLength() );
bool bQuoted = false;
bool bEscape = false;
bool bError = false;
for( sal_Int32 i=0; i < rTableName.getLength(); i++ )
OUString sTableName( rTableName );
// Documents written by older versions wrap a table name in single
// quotes (escaping inner quotes and backslashes with a backslash)
// when it contains spaces, dots, quotes or backslashes. Newer
// versions write the name unquoted, so only unescape the legacy form.
if( rTableName.startsWith("'") )
{
OUStringBuffer aBuffer( rTableName.getLength() );
bool bEscape = false;
bool bEndOfNameFound = false;
sal_Unicode c = rTableName[i];
switch( c )
for( sal_Int32 i = 1; i < rTableName.getLength() && !bEndOfNameFound; ++i )
{
case '\'':
sal_Unicode c = rTableName[i];
if( bEscape )
{
aBuffer.append( c );
bEscape = false;
}
else if( bQuoted )
{
bEndOfNameFound = true;
}
else if( 0 == i )
{
bQuoted = true;
}
else
{
bError = true;
}
break;
case '\\':
if( bEscape )
{
aBuffer.append( c );
bEscape = false;
}
else
{
else if( c == '\\' )
bEscape = true;
}
break;
case ' ':
case '.':
if( !bQuoted )
{
else if( c == '\'' )
bEndOfNameFound = true;
}
else
{
aBuffer.append( c );
bEscape = false;
}
break;
default:
{
aBuffer.append( c );
bEscape = false;
}
break;
}
if( bError || bEndOfNameFound )
break;
}
if( !bError )
{
OUString sTableName( aBuffer.makeStringAndClear() );
pOLENode->SetChartTableName( UIName(GetRenameMap().Get( XML_TEXT_RENAME_TYPE_TABLE, sTableName )) );
sTableName = aBuffer.makeStringAndClear();
}
pOLENode->SetChartTableName(
UIName(GetRenameMap().Get(XML_TEXT_RENAME_TYPE_TABLE, sTableName)) );
}
sal_Int64 nDrawAspect = 0;
sw/source/ui/table/instable.cxx
+1 -1
View File
@@ -65,7 +65,7 @@ IMPL_LINK(SwInsTableDlg, TextFilterHdl, OUString&, rTest, bool)
SwInsTableDlg::SwInsTableDlg(SwView& rView)
: SfxDialogController(rView.GetFrameWeld(), u"modules/swriter/ui/inserttable.ui"_ustr,
u"InsertTableDialog"_ustr)
, m_aTextFilter(u" .<>"_ustr)
, m_aTextFilter(u".<>"_ustr)
, m_pShell(&rView.GetWrtShell())
, m_nEnteredValRepeatHeaderNF(-1)
, m_aWndPreview(rView.GetWrtShell().IsCursorInTable() ? rView.GetWrtShell().IsTableRightToLeft()
sw/source/ui/table/tabledlg.cxx
+1 -13
View File
@@ -21,7 +21,7 @@
#include <o3tl/untaint.hxx>
#include <vcl/svapp.hxx>
#include <vcl/weld/Builder.hxx>
#include <vcl/weld/MessageDialog.hxx>
#include <vcl/weld/Window.hxx>
#include <svl/stritem.hxx>
#include <svl/intitem.hxx>
#include <editeng/keepitem.hxx>
@@ -37,7 +37,6 @@
#include <sfx2/htmlmode.hxx>
#include <sfx2/sfxdlg.hxx>
#include <strings.hrc>
#include <svx/strings.hrc>
#include <svx/dialmgr.hxx>
@@ -617,17 +616,6 @@ void SwFormatTablePage::ActivatePage( const SfxItemSet& rSet )
DeactivateRC SwFormatTablePage::DeactivatePage( SfxItemSet* _pSet )
{
//test the table name for spaces
OUString sTableName = m_xNameED->get_text();
if(sTableName.indexOf(' ') != -1)
{
std::unique_ptr<weld::MessageDialog> xInfoBox(Application::CreateMessageDialog(GetFrameWeld(),
VclMessageType::Info, VclButtonsType::Ok,
SwResId(STR_WRONG_TABLENAME)));
xInfoBox->run();
m_xNameED->grab_focus();
return DeactivateRC::KeepPage;
}
if(_pSet)
{
FillItemSet(_pSet);
sw/source/uibase/utlui/content.cxx
+1 -1
View File
@@ -3367,7 +3367,7 @@ IMPL_LINK(SwContentTree, EditedEntryHdl, const weld::TreeView::IterColText&, rIt
}
else if (ContentTypeId::TABLE == nType)
{
sForbiddenChars = " .<>";
sForbiddenChars = ".<>";
}
if (!sForbiddenChars.isEmpty())