Re: [Libreoffice] [REVIEW] RTF import patches for 3-4

[Michael Meeks]

Hi Cedric,

On Fri, 2011-07-08 at 15:06 +0200, Cedric Bosdonnat wrote:
 Here are two patches to review for the 3.4 branch. I know it's stupid to
 hack the RTF import filter... but the new one won't be in 3.4 ;)

The second patch - basic-handling-of-lines looks fine to me - ack'd for
-3-4. The first is a bit more interesting and/or requires some more
insight from Miklos I guess.

I don't see any other code doing this:

+// Remove the properties that have been parsed before in the paragraph
+GetAttrStack().pop_back();

in the RTF filter code. I see ReadHeaderFooter having a
save/clear/restore style thing [ is that what we want ? ].

So - I'm a tad more nervous there ;-) Also, AFAIR you're on vacation
from now, so I'll merged your 2nd patch myself :-)

Thanks,

Michael.

-- 
 michael.me...@novell.com  , Pseudo Engineer, itinerant idiot


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [REVIEW] RTF import patches for 3-4

[Miklos Vajna]

On Mon, Jul 11, 2011 at 11:45:33AM +0100, Michael Meeks 
michael.me...@novell.com wrote:
 Hi Cedric,
 
 On Fri, 2011-07-08 at 15:06 +0200, Cedric Bosdonnat wrote:
  Here are two patches to review for the 3.4 branch. I know it's stupid to
  hack the RTF import filter... but the new one won't be in 3.4 ;)
 
   The second patch - basic-handling-of-lines looks fine to me - ack'd for
 -3-4. The first is a bit more interesting and/or requires some more
 insight from Miklos I guess.
 
   I don't see any other code doing this:
 
 +// Remove the properties that have been parsed before in the 
 paragraph
 +GetAttrStack().pop_back();

Hi Michael,

[ Just from that it looks incorrect: if we have a stack that is pushed and
popped after each { and } (as RTF requires it), then doing a pop_back()
to reset the properties will cause a problem at the end (when the
importer tries to pop the last - already popped - element).

But given that I'm not really familiar with the old code and I'm sure
the above reasoning would result in an inmediate crash, just ignore it.
:) ]

Anyway, is the bug private on purpose? From the patches looks like it
has something with the old-style shapes for which I have no test
documents, as Word97+ no longer emits that syntax. At the moment these
Word 6.0/95-style drawing are completely ignored by the new importer -
in case you think it would not be a waste of time to support the old
syntax as well, then I would be interested in the test document.

Thanks.


pgpO3Xd7QF02p.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] [REVIEW] RTF import patches for 3-4

[Cedric Bosdonnat]

Hi all,

Here are two patches to review for the 3.4 branch. I know it's stupid to
hack the RTF import filter... but the new one won't be in 3.4 ;)

Thanks,

-- 
Cédric Bosdonnat
LibreOffice hacker
http://documentfoundation.org
OOo Eclipse Integration developer
http://cedric.bosdonnat.free.fr
From 49cfac3bdd9876496954ca1d52a714d6b2edd619 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= cedric.bosdonnat@free.fr
Date: Mon, 27 Jun 2011 16:46:16 +0200
Subject: [PATCH 1/2] n#695479: Remove properties when removing empty fly frame

---
 sw/source/filter/rtf/rtffly.cxx |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sw/source/filter/rtf/rtffly.cxx b/sw/source/filter/rtf/rtffly.cxx
index 3e02399..eb35f2a 100644
--- a/sw/source/filter/rtf/rtffly.cxx
+++ b/sw/source/filter/rtf/rtffly.cxx
@@ -1098,6 +1098,8 @@ void SwRTFParser::ReadFly( int nToken, SfxItemSet* pSet )
 // dann zerstoere den FlySave wieder.
 aFlyArr.DeleteAndDestroy( --nFlyArrCnt );
 
+// Remove the properties that have been parsed before in the paragraph
+GetAttrStack().pop_back();
 }
 else
 {
-- 
1.7.3.4

From b1e022057ba263f5233025e7c0d566dc91080227 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= cedric.bosdonnat@free.fr
Date: Fri, 8 Jul 2011 14:49:14 +0200
Subject: [PATCH 2/2] n#695479: basic handling of lines in RTF import

---
 sw/source/filter/rtf/swparrtf.cxx |   14 +++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/sw/source/filter/rtf/swparrtf.cxx b/sw/source/filter/rtf/swparrtf.cxx
index 1b5c076..49a6f77 100644
--- a/sw/source/filter/rtf/swparrtf.cxx
+++ b/sw/source/filter/rtf/swparrtf.cxx
@@ -1173,7 +1173,7 @@ void SwRTFParser::ReadShpTxt(String s)
 }
 
 /*
- * Very basic support for the Buchhalternase.
+ * Very basic support for the Z-line.
  */
 void SwRTFParser::ReadDrawingObject()
 {
@@ -1185,6 +1185,9 @@ void SwRTFParser::ReadDrawingObject()
 ::basegfx::B2DPoint aPoint;
 bool bPolygonActive(false);
 
+SwFmtHoriOrient aHori( 0, text::HoriOrientation::NONE, text::RelOrientation::PAGE_FRAME );
+SwFmtVertOrient aVert( 0, text::VertOrientation::NONE, text::RelOrientation::PAGE_FRAME );
+
 while (level0  IsParserWorking())
 {
 nToken = GetNextToken();
@@ -1196,6 +1199,12 @@ void SwRTFParser::ReadDrawingObject()
 case '{':
 level++;
 break;
+case RTF_DOBXMARGIN:
+aHori.SetRelationOrient( text::RelOrientation::PAGE_PRINT_AREA );
+break;
+case RTF_DOBYMARGIN:
+aVert.SetRelationOrient( text::RelOrientation::PAGE_PRINT_AREA );
+break;
 case RTF_DPX:
 aRect.setX(nTokenValue);
 break;
@@ -1208,6 +1217,7 @@ void SwRTFParser::ReadDrawingObject()
 case RTF_DPYSIZE:
 aRect.setHeight(nTokenValue);
 break;
+case RTF_DPLINE:
 case RTF_DPPOLYCOUNT:
 bPolygonActive = true;
 break;
@@ -1244,9 +1254,7 @@ void SwRTFParser::ReadDrawingObject()
 aAnchor.SetAnchor( pPam-GetPoint() );
 aFlySet.Put( aAnchor );
 
-SwFmtHoriOrient aHori( 0, text::HoriOrientation::NONE, text::RelOrientation::PAGE_FRAME );
 aFlySet.Put( aHori );
-SwFmtVertOrient aVert( 0, text::VertOrientation::NONE, text::RelOrientation::PAGE_FRAME );
 aFlySet.Put( aVert );
 
 pDoc-GetOrCreateDrawModel();
-- 
1.7.3.4

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice