handle scope of w:pPrChange and w:rPrChange properly (bnc#821804)
Redlines changing formatting of runs and paragraphs are valid for the entire
run/paragraph, not just their existence in the XML. So store them
in the matching contexts, which will care of it, instead of the endtrackchange
stuff.
Change-Id: Ie583e4be14e8df95829852bfbbbe25aa0684f02e
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 4ef5e71..ea439c3 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -2222,9 +2222,10 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, PropertyMapPtr rContext )
}
break;
case NS_ooxml::LN_endtrackchange:
m_pImpl->RemoveCurrentRedline( );
m_pImpl->RemoveTopRedline();
break;
case NS_ooxml::LN_CT_RPrChange_rPr:
{
// Push all the current 'Character' properties to the stack, so that we don't store them
// as 'tracked changes' by mistake
m_pImpl->PushProperties(CONTEXT_CHARACTER);
@@ -2232,19 +2233,19 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, PropertyMapPtr rContext )
// Resolve all the properties that are under the 'rPrChange'->'rPr' XML node
resolveSprmProps(*this, rSprm );
if (m_pImpl->GetTopContext())
{
// Get all the properties that were processed in the 'rPrChange'->'rPr' XML node
uno::Sequence< beans::PropertyValue > currentRedlineRevertProperties = m_pImpl->GetTopContext()->GetPropertyValues();
// Store these properties in the current redline object
m_pImpl->SetCurrentRedlineRevertProperties( currentRedlineRevertProperties );
}
// Get all the properties that were processed in the 'rPrChange'->'rPr' XML node
uno::Sequence< beans::PropertyValue > currentRedlineRevertProperties = m_pImpl->GetTopContext()->GetPropertyValues();
// Pop back out the character properties that were on the run
m_pImpl->PopProperties(CONTEXT_CHARACTER);
// Store these properties in the current redline object (do it after the PopProperties() above, since
// otherwise it'd be stored in the content dropped there).
m_pImpl->SetCurrentRedlineRevertProperties( currentRedlineRevertProperties );
}
break;
case NS_ooxml::LN_CT_PPrChange_pPr:
{
// Push all the current 'Paragraph' properties to the stack, so that we don't store them
// as 'tracked changes' by mistake
m_pImpl->PushProperties(CONTEXT_PARAGRAPH);
@@ -2252,17 +2253,16 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, PropertyMapPtr rContext )
// Resolve all the properties that are under the 'pPrChange'->'pPr' XML node
resolveSprmProps(*this, rSprm );
if (m_pImpl->GetTopContext())
{
// Get all the properties that were processed in the 'pPrChange'->'pPr' XML node
uno::Sequence< beans::PropertyValue > currentRedlineRevertProperties = m_pImpl->GetTopContext()->GetPropertyValues();
// Store these properties in the current redline object
m_pImpl->SetCurrentRedlineRevertProperties( currentRedlineRevertProperties );
}
// Get all the properties that were processed in the 'pPrChange'->'pPr' XML node
uno::Sequence< beans::PropertyValue > currentRedlineRevertProperties = m_pImpl->GetTopContext()->GetPropertyValues();
// Pop back out the character properties that were on the run
m_pImpl->PopProperties(CONTEXT_PARAGRAPH);
// Store these properties in the current redline object (do it after the PopProperties() above, since
// otherwise it'd be stored in the content dropped there).
m_pImpl->SetCurrentRedlineRevertProperties( currentRedlineRevertProperties );
}
break;
case NS_ooxml::LN_object:
{
@@ -3444,7 +3444,7 @@ void DomainMapper::HandleRedline( Sprm& rSprm )
{
sal_uInt32 nSprmId = rSprm.getId();
m_pImpl->AddNewRedline( );
m_pImpl->AddNewRedline( nSprmId );
if (nSprmId == NS_ooxml::LN_CT_PPr_pPrChange)
{
@@ -3484,6 +3484,7 @@ void DomainMapper::HandleRedline( Sprm& rSprm )
default: OSL_FAIL( "redline token other than mod, ins, del or table row" ); break;
}
m_pImpl->EndParaMarkerChange( );
m_pImpl->SetCurrentRedlineIsRead();
}
} //namespace dmapper
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 24889e5..eb08875 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1592,7 +1592,7 @@ void DomainMapper_Impl::PushFootOrEndnote( bool bIsFootnote )
}
void DomainMapper_Impl::CreateRedline(uno::Reference<text::XTextRange> const& xRange,
RedlineParamsPtr& pRedline)
RedlineParamsPtr pRedline)
{
if ( pRedline.get( ) )
{
@@ -1646,21 +1646,25 @@ void DomainMapper_Impl::CheckParaMarkerRedline( uno::Reference< text::XTextRange
void DomainMapper_Impl::CheckRedline( uno::Reference< text::XTextRange > const& xRange )
{
// Writer core "officially" does not like overlapping redlines, and its UNO interface is stupid enough
// to not prevent that. However, in practice in fact everything appears to work fine (except for the debug warnings
// about redline table corruption, which may possibly be harmless in reality). So leave this as it is, since this
// is a better representation of how the changes happened. If this will ever become a problem, overlapping redlines
// will need to be merged into one, just like doing the changes in the UI does, which will lose some information
// (and so if that happens, it may be better to fix Writer).
// Create the redlines here from lowest (formats) to highest (inserts/removals) priority, since the last one is
// what Writer presents graphically, so this will show deletes as deleted text and not as just formatted text being there.
if( GetTopContextOfType(CONTEXT_PARAGRAPH) != NULL )
for( std::vector<RedlineParamsPtr>::const_iterator it = GetTopContextOfType(CONTEXT_PARAGRAPH)->Redlines().begin();
it != GetTopContextOfType(CONTEXT_PARAGRAPH)->Redlines().end(); ++it )
CreateRedline( xRange, *it );
if( GetTopContextOfType(CONTEXT_CHARACTER) != NULL )
for( std::vector<RedlineParamsPtr>::const_iterator it = GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().begin();
it != GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().end(); ++it )
CreateRedline( xRange, *it );
std::vector<RedlineParamsPtr>::iterator pIt = m_aRedlines.top().begin( );
std::vector< RedlineParamsPtr > aCleaned;
for (; pIt != m_aRedlines.top().end( ); ++pIt )
{
CreateRedline( xRange, *pIt );
// Adding the non-mod redlines to the temporary vector
if ( pIt->get( ) )
{
if (((*pIt)->m_nToken & 0xffff) != XML_mod && ((*pIt)->m_nToken & 0xffff) != XML_ParagraphFormat)
aCleaned.push_back(*pIt);
}
}
m_aRedlines.top().swap( aCleaned );
}
void DomainMapper_Impl::StartParaMarkerChange( )
@@ -1671,6 +1675,7 @@ void DomainMapper_Impl::StartParaMarkerChange( )
void DomainMapper_Impl::EndParaMarkerChange( )
{
m_bIsParaMarkerChange = false;
m_currentRedline.reset();
}
@@ -4608,36 +4613,43 @@ bool DomainMapper_Impl::ExecuteFrameConversion()
return bRet;
}
void DomainMapper_Impl::AddNewRedline( )
void DomainMapper_Impl::AddNewRedline( sal_uInt32 sprmId )
{
RedlineParamsPtr pNew( new RedlineParams );
pNew->m_nToken = XML_mod;
if ( !m_bIsParaMarkerChange )
{
m_aRedlines.top().push_back( pNew );
// <w:rPrChange> applies to the whole <w:r>, <w:pPrChange> applies to the whole <w:p>,
// so keep those two in CONTEXT_CHARACTERS and CONTEXT_PARAGRAPH, which will take
// care of their scope (i.e. when they should be used and discarded).
// Let's keep the rest the same way they used to be handled (explictly dropped
// from a global stack by endtrackchange), but quite possibly they should not be handled
// that way either (I don't know).
if( sprmId == NS_ooxml::LN_EG_RPrContent_rPrChange )
GetTopContextOfType( CONTEXT_CHARACTER )->Redlines().push_back( pNew );
else if( sprmId == NS_ooxml::LN_CT_PPr_pPrChange )
GetTopContextOfType( CONTEXT_PARAGRAPH )->Redlines().push_back( pNew );
else
m_aRedlines.top().push_back( pNew );
}
else
{
m_pParaMarkerRedline.swap( pNew );
m_pParaMarkerRedline = pNew;
}
// Newly read data will go into this redline.
m_currentRedline = pNew;
}
RedlineParamsPtr DomainMapper_Impl::GetTopRedline( )
void DomainMapper_Impl::SetCurrentRedlineIsRead()
{
RedlineParamsPtr pResult;
if ( !m_bIsParaMarkerChange && m_aRedlines.top().size( ) > 0 )
pResult = m_aRedlines.top().back( );
else if ( m_bIsParaMarkerChange )
pResult = m_pParaMarkerRedline;
return pResult;
m_currentRedline.reset();
}
sal_Int32 DomainMapper_Impl::GetCurrentRedlineToken( )
{
sal_Int32 nToken = 0;
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
nToken = pCurrent->m_nToken;
assert( m_currentRedline.get());
nToken = m_currentRedline->m_nToken;
return nToken;
}
@@ -4645,9 +4657,8 @@ void DomainMapper_Impl::SetCurrentRedlineAuthor( const OUString& sAuthor )
{
if (!m_xAnnotationField.is())
{
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
pCurrent->m_sAuthor = sAuthor;
assert( m_currentRedline.get());
m_currentRedline->m_sAuthor = sAuthor;
}
else
m_xAnnotationField->setPropertyValue("Author", uno::makeAny(sAuthor));
@@ -4663,9 +4674,8 @@ void DomainMapper_Impl::SetCurrentRedlineDate( const OUString& sDate )
{
if (!m_xAnnotationField.is())
{
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
pCurrent->m_sDate = sDate;
assert( m_currentRedline.get());
m_currentRedline->m_sDate = sDate;
}
else
m_xAnnotationField->setPropertyValue("DateTimeValue", uno::makeAny(ConversionHelper::ConvertDateStringToDateTime(sDate)));
@@ -4679,46 +4689,46 @@ void DomainMapper_Impl::SetCurrentRedlineId( sal_Int32 sId )
}
else
{
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
pCurrent->m_nId = sId;
// This should be an assert, but somebody had the smart idea to reuse this function also for comments and whatnot,
// and in some cases the id is actually not handled, which may be in fact a bug.
SAL_WARN( "writerfilter", !m_currentRedline.get());
if( m_currentRedline.get())
m_currentRedline->m_nId = sId;
}
}
void DomainMapper_Impl::SetCurrentRedlineToken( sal_Int32 nToken )
{
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
pCurrent->m_nToken = nToken;
assert( m_currentRedline.get());
m_currentRedline->m_nToken = nToken;
}
void DomainMapper_Impl::SetCurrentRedlineRevertProperties( const uno::Sequence<beans::PropertyValue>& aProperties )
{
RedlineParamsPtr pCurrent( GetTopRedline( ) );
if ( pCurrent.get( ) )
pCurrent->m_aRevertProperties = aProperties;
assert( m_currentRedline.get());
m_currentRedline->m_aRevertProperties = aProperties;
}
void DomainMapper_Impl::RemoveCurrentRedline( )
// This removes only the last redline stored here, those stored in contexts are automatically removed when
// the context is destroyed.
void DomainMapper_Impl::RemoveTopRedline( )
{
if ( m_aRedlines.top().size( ) > 0 )
{
m_aRedlines.top().pop_back( );
}
assert( m_aRedlines.top().size( ) > 0 );
m_aRedlines.top().pop_back( );
m_currentRedline.reset();
}
void DomainMapper_Impl::ResetParaMarkerRedline( )
{
if ( m_pParaMarkerRedline.get( ) )
{
RedlineParamsPtr pEmpty;
m_pParaMarkerRedline.swap( pEmpty );
m_pParaMarkerRedline.reset();
m_currentRedline.reset();
}
}
void DomainMapper_Impl::ApplySettingsTable()
{
if (m_pSettingsTable && m_xTextFactory.is())
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 0407a08..817510c 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -250,20 +250,6 @@ struct AnnotationPosition
};
typedef boost::unordered_map< sal_Int32, AnnotationPosition > AnnotationPositions_t;
struct RedlineParams
{
OUString m_sAuthor;
OUString m_sDate;
sal_Int32 m_nId;
sal_Int32 m_nToken;
/// This can hold properties of runs that had formatted 'track changes' properties
css::uno::Sequence<css::beans::PropertyValue> m_aRevertProperties;
};
typedef boost::shared_ptr< RedlineParams > RedlineParamsPtr;
struct LineNumberSettings
{
bool bIsOn;
@@ -392,6 +378,8 @@ private:
// Redline stack
std::stack< std::vector< RedlineParamsPtr > > m_aRedlines;
// The redline currently read, may be also stored by a context instead of m_aRedlines.
RedlineParamsPtr m_currentRedline;
RedlineParamsPtr m_pParaMarkerRedline;
bool m_bIsParaMarkerChange;
@@ -466,7 +454,7 @@ public:
}
void SetDocumentSettingsProperty( const OUString& rPropName, const css::uno::Any& rValue );
void CreateRedline( ::com::sun::star::uno::Reference< ::com::sun::star::text::XTextRange > const& xRange, RedlineParamsPtr& pRedline );
void CreateRedline( ::com::sun::star::uno::Reference< ::com::sun::star::text::XTextRange > const& xRange, RedlineParamsPtr pRedline );
void CheckParaMarkerRedline( ::com::sun::star::uno::Reference< ::com::sun::star::text::XTextRange > const& xRange );
@@ -725,9 +713,7 @@ public:
);
bool ExecuteFrameConversion();
void AddNewRedline( );
RedlineParamsPtr GetTopRedline( );
void AddNewRedline( sal_uInt32 sprmId );
sal_Int32 GetCurrentRedlineToken( );
void SetCurrentRedlineAuthor( const OUString& sAuthor );
@@ -735,7 +721,8 @@ public:
void SetCurrentRedlineId( sal_Int32 nId );
void SetCurrentRedlineToken( sal_Int32 nToken );
void SetCurrentRedlineRevertProperties( const css::uno::Sequence<css::beans::PropertyValue>& aProperties );
void RemoveCurrentRedline( );
void SetCurrentRedlineIsRead();
void RemoveTopRedline( );
void ResetParaMarkerRedline( );
void SetCurrentRedlineInitials( const OUString& sInitials );
bool IsFirstRun() { return m_bIsFirstRun;}
diff --git a/writerfilter/source/dmapper/PropertyMap.hxx b/writerfilter/source/dmapper/PropertyMap.hxx
index dcd313d..dd6d2a7 100644
--- a/writerfilter/source/dmapper/PropertyMap.hxx
+++ b/writerfilter/source/dmapper/PropertyMap.hxx
@@ -74,6 +74,18 @@ enum GrabBagType
CHAR_GRAB_BAG
};
struct RedlineParams
{
OUString m_sAuthor;
OUString m_sDate;
sal_Int32 m_nId;
sal_Int32 m_nToken;
/// This can hold properties of runs that had formatted 'track changes' properties
css::uno::Sequence<css::beans::PropertyValue> m_aRevertProperties;
};
typedef boost::shared_ptr< RedlineParams > RedlineParamsPtr;
class PropValue
{
css::uno::Any m_aValue;
@@ -105,6 +117,9 @@ class PropertyMap
std::map< PropertyIds, PropValue > m_vMap;
typedef std::map<PropertyIds,PropValue>::const_iterator MapIterator;
std::vector< RedlineParamsPtr > m_aRedlines;
protected:
void Invalidate()
{
@@ -151,6 +166,10 @@ public:
void SetFootnoteFontName( const OUString& rSet ) { m_sFootnoteFontName = rSet;}
virtual void insertTableProperties( const PropertyMap* );
const std::vector< RedlineParamsPtr >& Redlines() const { return m_aRedlines; }
std::vector< RedlineParamsPtr >& Redlines() { return m_aRedlines; }
#ifdef DEBUG_DOMAINMAPPER
void printProperties();
#endif
diff --git a/writerfilter/source/ooxml/model.xml b/writerfilter/source/ooxml/model.xml
index e2cfab9..febe5b1 100644
--- a/writerfilter/source/ooxml/model.xml
+++ b/writerfilter/source/ooxml/model.xml
@@ -17338,9 +17338,6 @@
</resource>
<resource name="CT_RPrChange" resource="Properties">
<element name="rPr" tokenid="ooxml:CT_RPrChange_rPr"/>
<action name="end" action="tokenproperty"/>
<action name="end" action="sendPropertiesWithId" sendtokenid="ooxml:endtrackchange"/>
<action name="end" action="clearProps"/>
</resource>
<resource name="CT_ParaRPrChange" resource="Properties">
<element name="rPr" tokenid="ooxml:CT_ParaRPrChange_rPr"/>
