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.l...@collabora.com>
AuthorDate: Thu Nov 1 15:38:58 2018 +0300
Commit:     Justin Luth <justin_l...@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_l...@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.
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to