[Libreoffice-commits] core.git: toolkit/source vcl/qa
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Mon Nov 5 05:03:15 UTC 2018
toolkit/source/helper/formpdfexport.cxx | 71 +++++------
vcl/qa/cppunit/pdfexport/data/tdf118244_radioButtonGroup.odt |binary
vcl/qa/cppunit/pdfexport/pdfexport.cxx | 39 ++++++
3 files changed, 77 insertions(+), 33 deletions(-)
New commits:
commit 80400973e06e08f0c1ab0b9a86418c1bcc4bbd5c
Author: Justin Luth <justin.luth at collabora.com>
AuthorDate: Thu Nov 1 15:38:58 2018 +0300
Commit: Justin Luth <justin_luth at sil.org>
CommitDate: Mon Nov 5 06:02:23 2018 +0100
tdf#118244 pdfexport: radio buttons use groupname now
The previous implementation grouped radio buttons if their
object name was the same. Likely this is a very old implementation,
because the current radio buttons have a groupname property which
links them together (although that too needed fixing in doc/docx),
and their object names are unique.
The old implementation still works - so that still needs to be
supported, but I think I'll do that in a separate patch, so
that it can be easily reverted if the old implementation is
deprecated.
Edge cases tested:
-groupID of 0 works fine - doesn't have to be 1-based.
-empty group name works fine (but breaks the old impl).
-writer, calc,
Change-Id: I84aebdac18b9edfa5ffcbfb23c15d0f37fcd47d1
Reviewed-on: https://gerrit.libreoffice.org/62742
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth at sil.org>
diff --git a/toolkit/source/helper/formpdfexport.cxx b/toolkit/source/helper/formpdfexport.cxx
index d0399e6f7a39..dd225934a790 100644
--- a/toolkit/source/helper/formpdfexport.cxx
+++ b/toolkit/source/helper/formpdfexport.cxx
@@ -20,6 +20,8 @@
#include <memory>
#include <toolkit/helper/formpdfexport.hxx>
+#include <tools/diagnose_ex.h>
+#include <unordered_map>
#include <com/sun/star/container/XIndexAccess.hpp>
#include <com/sun/star/container/XNameAccess.hpp>
@@ -130,7 +132,7 @@ namespace toolkitform
// host document makes it somewhat difficult ...
// Problem is that two form radio buttons belong to the same group if
// - they have the same parent
- // - AND they have the same name
+ // - AND they have the same group name
// This implies that we need some knowledge about (potentially) *all* radio button
// groups in the document.
@@ -156,29 +158,12 @@ namespace toolkitform
::std::vector< sal_Int32 > aPath;
Reference< XInterface > xNormalizedLookup( _rxRadioModel, UNO_QUERY );
- OUString sRadioGroupName;
- OSL_VERIFY( _rxRadioModel->getPropertyValue( FM_PROP_NAME ) >>= sRadioGroupName );
-
Reference< XIndexAccess > xCurrentContainer( xRoot );
sal_Int32 nStartWithChild = 0;
sal_Int32 nGroupsEncountered = 0;
do
{
- Reference< XNameAccess > xElementNameAccess( xCurrentContainer, UNO_QUERY );
- OSL_ENSURE( xElementNameAccess.is(), "determineRadioGroupId: no name container?" );
- if ( !xElementNameAccess.is() )
- return -1;
-
- if ( nStartWithChild == 0 )
- { // we encounter this container the first time. In particular, we did not
- // just step up
- nGroupsEncountered += xElementNameAccess->getElementNames().getLength();
- // this is way too much: Not all of the elements in the current container
- // may form groups, especially if they're forms. But anyway, this number is
- // sufficient for our purpose. Finally, the container contains *at most*
- // that much groups
- }
-
+ std::unordered_map<OUString,sal_Int32> GroupNameMap;
sal_Int32 nCount = xCurrentContainer->getCount();
sal_Int32 i;
for ( i = nStartWithChild; i < nCount; ++i )
@@ -204,27 +189,47 @@ namespace toolkitform
if ( xElement.get() == xNormalizedLookup.get() )
{
- // look up the name of the radio group in the list of all element names
- Sequence< OUString > aElementNames( xElementNameAccess->getElementNames() );
- const OUString* pElementNames = aElementNames.getConstArray();
- const OUString* pElementNamesEnd = pElementNames + aElementNames.getLength();
- while ( pElementNames != pElementNamesEnd )
+ // Our radio button is in this container.
+ // Now take the time to ID this container's groups and return the button's groupId
+ for ( i = 0; i < nCount; ++i )
{
- if ( *pElementNames == sRadioGroupName )
+ try
{
- sal_Int32 nLocalGroupIndex = pElementNames - aElementNames.getConstArray();
- OSL_ENSURE( nLocalGroupIndex < xElementNameAccess->getElementNames().getLength(),
- "determineRadioGroupId: inconsistency!" );
-
- sal_Int32 nGlobalGroupId = nGroupsEncountered - xElementNameAccess->getElementNames().getLength() + nLocalGroupIndex;
- return nGlobalGroupId;
+ xElement.set( xCurrentContainer->getByIndex( i ), UNO_QUERY_THROW );
+ Reference< XServiceInfo > xModelSI( xElement, UNO_QUERY_THROW );
+ if ( xModelSI->supportsService("com.sun.star.awt.UnoControlRadioButtonModel") )
+ {
+ Reference< XPropertySet > aProps( xElement, UNO_QUERY_THROW );
+
+ OUString sGroupName;
+ aProps->getPropertyValue("GroupName") >>= sGroupName;
+ // map: unique key is the group name, so attempts to add a different ID value
+ // for an existing group are ignored - keeping the first ID - perfect for this scenario.
+ GroupNameMap.emplace( sGroupName, nGroupsEncountered + i );
+
+ if ( xElement.get() == xNormalizedLookup.get() )
+ return GroupNameMap[sGroupName];
+ }
+ }
+ catch( uno::Exception& )
+ {
+ DBG_UNHANDLED_EXCEPTION("toolkit");
}
- ++pElementNames;
}
- OSL_FAIL( "determineRadioGroupId: did not find the radios element name!" );
+ SAL_WARN("toolkit","determineRadioGroupId: did not find the radios element's group!" );
}
}
+ // we encounter this container the first time. In particular, we did not just step up
+ if ( nStartWithChild == 0 )
+ {
+ // Our control wasn't in this container, so consider every item to be a possible unique group.
+ // This is way too much: Not all of the elements in the current container will form groups.
+ // But anyway, this number is sufficient for our purpose, since sequential group ids are not required.
+ // Ultimately, the container contains *at most* this many groups.
+ nGroupsEncountered += nCount;
+ }
+
if ( i >= nCount )
{
// the loop terminated because there were no more elements
diff --git a/vcl/qa/cppunit/pdfexport/data/tdf118244_radioButtonGroup.odt b/vcl/qa/cppunit/pdfexport/data/tdf118244_radioButtonGroup.odt
new file mode 100644
index 000000000000..caabc4987c30
Binary files /dev/null and b/vcl/qa/cppunit/pdfexport/data/tdf118244_radioButtonGroup.odt differ
diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
index 1ee110972865..71fd198a0e35 100644
--- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx
+++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
@@ -94,6 +94,7 @@ public:
void testTdf99680();
void testTdf99680_2();
void testTdf108963();
+ void testTdf118244_radioButtonGroup();
#if HAVE_MORE_FONTS
/// Test writing ToUnicode CMAP for LTR ligatures.
void testTdf115117_1();
@@ -132,6 +133,7 @@ public:
CPPUNIT_TEST(testTdf99680);
CPPUNIT_TEST(testTdf99680_2);
CPPUNIT_TEST(testTdf108963);
+ CPPUNIT_TEST(testTdf118244_radioButtonGroup);
#if HAVE_MORE_FONTS
CPPUNIT_TEST(testTdf115117_1);
CPPUNIT_TEST(testTdf115117_1a);
@@ -872,6 +874,43 @@ void PdfExportTest::testTdf108963()
CPPUNIT_ASSERT_EQUAL(1, nYellowPathCount);
}
+void PdfExportTest::testTdf118244_radioButtonGroup()
+{
+ vcl::filter::PDFDocument aDocument;
+ load("tdf118244_radioButtonGroup.odt", aDocument);
+
+ // The document has one page.
+ std::vector<vcl::filter::PDFObjectElement*> aPages = aDocument.GetPages();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPages.size());
+
+ // There are eight radio buttons.
+ auto pAnnots = dynamic_cast<vcl::filter::PDFArrayElement*>(aPages[0]->Lookup("Annots"));
+ CPPUNIT_ASSERT(pAnnots);
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("# of radio buttons",static_cast<size_t>(8), pAnnots->GetElements().size());
+
+ sal_uInt32 nRadioGroups = 0;
+ for ( const auto& aElement : aDocument.GetElements() )
+ {
+ auto pObject = dynamic_cast<vcl::filter::PDFObjectElement*>(aElement.get());
+ if ( !pObject )
+ continue;
+ auto pType = dynamic_cast<vcl::filter::PDFNameElement*>(pObject->Lookup("FT"));
+ if ( pType && pType->GetValue() == "Btn" )
+ {
+ auto pKids = dynamic_cast<vcl::filter::PDFArrayElement*>(pObject->Lookup("Kids"));
+ if ( pKids )
+ {
+ size_t expectedSize = 2;
+ ++nRadioGroups;
+ if ( nRadioGroups == 2 )
+ expectedSize = 5;
+ CPPUNIT_ASSERT_EQUAL(expectedSize, pKids->GetElements().size());
+ }
+ }
+ }
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("# of radio groups", sal_uInt32(2), nRadioGroups);
+}
+
#if HAVE_MORE_FONTS
// This requires Carlito font, if it is missing the test will most likely
// fail.
More information about the Libreoffice-commits
mailing list