DOCX: merged comments

[Pavel Janík <pa...@janik.cz>]

Hi,

today I was shown the problem which is described in the issue

https://issues.apache.org/ooo/show_bug.cgi?id=113059

This issue is fixed in LibreOffice and thus I (after spending some hours xmllint'ing OOXML's comments.xml and debugging writerfilter) isolated and improved the fix for AOO and attached sample patch to the issue.

Any volunteers to review/intergation/QA?

DOCX comments need more love: no author is shown (DOCS contains initials and author's name) and the date of the change is ignored and current date is used... Any takers?
-- 
Pavel Janík

Re: DOCX: merged comments

[Pavel Janík <Pa...@Janik.cz>]

On Nov 30, 2012, at 9:22 AM, Pavel Janík wrote:

> So yes, the patch is influenced by similar LO change and Apache OpenOffice project has to receive original author's ack. Accorging to LibreOffice' VCS, it is Cedric. I'll ask him for acknowledgement.

Cedric declined, so if anyone is interested to recreate the change from scratch, go for it. It is very simple one. I won't do it myself to prevent legal issues.

The problem is very simple:

Grab some DOCX document containing more than two comments (so you can check the results). Unzip it.

Investigate Comments part (see Office Open XML Part 1 - Fundamentals And Markup Language Reference.pdf for more details).

Investigate 

xmllint --format word/comments.xml

Grep for comments:

bash-3.2$ xmllint --format word/comments.xml | grep "<w:comment"

Comments are numbered:

  <w:comment w:id="0" w:author="Deborah" w:date="2010-11-19T16:41:00Z" w:initials="D">
  <w:comment w:id="1" w:author="Deborah" w:date="2010-11-19T14:41:00Z" w:initials="D">
  <w:comment w:id="2" w:author="Deborah" w:date="2010-11-19T14:43:00Z" w:initials="D">
  <w:comment w:id="3" w:author="Deborah" w:date="2010-11-19T14:49:00Z" w:initials="D">
  <w:comment w:id="4" w:author="Deborah" w:date="2010-11-19T14:53:00Z" w:initials="D">
  <w:comment w:id="5" w:author="Deborah" w:date="2010-11-19T14:51:00Z" w:initials="D">

According to wml.xsd, attribute id (w:id) is:

  <xsd:complexType name="CT_Markup">
    <xsd:attribute name="id" type="ST_DecimalNumber" use="required"/>
  </xsd:complexType>

but our model (writerfilter/source/ooxml/model.xml) contains:

        <attribute name="id">
          <text/>

which means it is being worked on as a string which is wrong. It should be ST_DecimalNumber.

And thats all.
-- 
Pavel Janík

Re: DOCX: merged comments

[Pavel Janík <Pa...@Janik.cz>]

On Nov 30, 2012, at 4:00 AM, Shenfeng Liu wrote:

> Pavel,
>  Thanks for your patch! But I'd like to clarify one thing: did you take
> the fix out from LO?

First I found the problem myself, fixed it only for comments without e.g. renaming variables (wrongly using "r" vars for strings etc., but it worked). And then I wanted to find another sample document which I found in LO bugzilla. And there was patch which was more extended, used proper naming of variables etc. I unified both of them, fixed other debugging code which was not properly fixed in LO patch (I have had debugging and WaE turned on in my tree because I investigated the problem before).

My primary goal was not to create patch for integration but to create patch which my customer is able to use immediatelly.

So yes, the patch is influenced by similar LO change and Apache OpenOffice project has to receive original author's ack. Accorging to LibreOffice' VCS, it is Cedric. I'll ask him for acknowledgement.
-- 
Pavel Janík

Re: DOCX: merged comments

[Shenfeng Liu <li...@gmail.com>]

Pavel,
  Thanks for your patch! But I'd like to clarify one thing: did you take
the fix out from LO?
  My understanding is that due to the license limitation, unfortunately we
can not take the code from LO.

- Shenfeng (Simon)


2012/11/30 Pavel Janík <pa...@janik.cz>

> Hi,
>
> today I was shown the problem which is described in the issue
>
> https://issues.apache.org/ooo/show_bug.cgi?id=113059
>
> This issue is fixed in LibreOffice and thus I (after spending some hours
> xmllint'ing OOXML's comments.xml and debugging writerfilter) isolated and
> improved the fix for AOO and attached sample patch to the issue.
>
> Any volunteers to review/intergation/QA?
>
> DOCX comments need more love: no author is shown (DOCS contains initials
> and author's name) and the date of the change is ignored and current date
> is used... Any takers?
> --
> Pavel Janík
>
>
>
>