[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: Bug fix: If tables are inside Kword document,
From:       "Thomas Zander" <zander () kde ! org>
Date:       2010-06-21 14:03:58
Message-ID: 20100621140358.14610.26925 () localhost
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> On 2010-06-17 18:55:57, Thomas Zander wrote:
> > The empty paragraph is correct and expected and because we want to stay=
 compatible with QTextDocument objects populated with something other than =
the loader this change is incorrect.
> > =

> > I think I mentioned on IRC that the paragraph after the table should be=
 there but should not have any height. If the layout code gives it some hei=
ght then this should be fixed. A unit test would be good as I am pretty sur=
e this used to work in an earlier KOffice version.
> =

> Matus Hanzes wrote:
>     This is not about empty paragraph after the table, this is about empt=
y paragraph added to the end of document.
>     =

>     To explain it in more detail.
>     When table is added to the document, frame is added to the document w=
hich contains table and paragraph and another paragraph is added to the end=
 of document. (table + paragraph after table + paragraph at the end of docu=
ment are added).
>     =

>     Problem with the paragraph added to the end of document is that it ha=
s all the properties than the table.
>     So when the table has master page style the paragraph at the end of d=
ocument has it too.
>     And empty page is added to the end of document. This cannot be fixed =
with 0 height.
>     =

>     Because I can't identify this paragraph in layout.cpp I have set the =
visibility in KoTextLoader::loadTable.
>     =

>     =

>

> Because I can't identify this paragraph in layout.cpp I have set the visi=
bility in KoTextLoader::loadTable.

But you agree with my first comment that the solution is wrong, right?

Maybe you can close the review request and work on a proper solution.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4366/#review6169
-----------------------------------------------------------


On 2010-06-17 13:09:24, Matus Hanzes wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4366/
> -----------------------------------------------------------
> =

> (Updated 2010-06-17 13:09:24)
> =

> =

> Review request for KOffice.
> =

> =

> Summary
> -------
> =

> When tables are added to the QTextDocument not only table frame is added =
to the QTextDocument, but empty paragraph is added to the end of QTextDocum=
ent.
> =

> I didn't find a way how to identify this empty paragraphs, so I have set =
their visibility to false in KoTextLoader::loadTable, when they are added t=
o QTextDocument.
> =

> =

> Diffs
> -----
> =

>   trunk/koffice/libs/kotext/opendocument/KoTextLoader.cpp 1139065 =

>   trunk/koffice/plugins/textshape/Layout.cpp 1139065 =

>   trunk/koffice/plugins/textshape/TextShape.cpp 1139065 =

> =

> Diff: http://reviewboard.kde.org/r/4366/diff
> =

> =

> Testing
> -------
> =

> =

> Thanks,
> =

> Matus
> =

>


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://reviewboard.kde.org/r/4366/">http://reviewboard.kde.org/r/4366/</a>
  </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On June 17th, 2010, 6:55 p.m., <b>Thomas \
Zander</b> wrote:</p>  <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">  <pre>The empty paragraph is correct and expected and \
because we want to stay compatible with QTextDocument objects populated with \
something other than the loader this change is incorrect.

I think I mentioned on IRC that the paragraph after the table should be there but \
should not have any height. If the layout code gives it some height then this should \
be fixed. A unit test would be good as I am pretty sure this used to work in an \
earlier KOffice version.</pre>  </blockquote>




 <p>On June 18th, 2010, 7:29 a.m., <b>Matus Hanzes</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre>This is not about empty paragraph after the table, this is about empty \
paragraph added to the end of document.

To explain it in more detail.
When table is added to the document, frame is added to the document which contains \
table and paragraph and another paragraph is added to the end of document. (table + \
paragraph after table + paragraph at the end of document are added).

Problem with the paragraph added to the end of document is that it has all the \
properties than the table. So when the table has master page style the paragraph at \
the end of document has it too. And empty page is added to the end of document. This \
cannot be fixed with 0 height.

Because I can&#39;t identify this paragraph in layout.cpp I have set the visibility \
in KoTextLoader::loadTable.


</pre>
 </blockquote>








</blockquote>

<pre>&gt; Because I can&#39;t identify this paragraph in layout.cpp I have set the \
visibility in KoTextLoader::loadTable.

But you agree with my first comment that the solution is wrong, right?

Maybe you can close the review request and work on a proper solution.</pre>
<br />








<p>- Thomas</p>


<br />
<p>On June 17th, 2010, 1:09 p.m., Matus Hanzes wrote:</p>




<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for KOffice.</div>
<div>By Matus Hanzes.</div>


<p style="color: grey;"><i>Updated 2010-06-17 13:09:24</i></p>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0;">When tables are added to the QTextDocument not \
only table frame is added to the QTextDocument, but empty paragraph is added to the \
end of QTextDocument.

I didn&#39;t find a way how to identify this empty paragraphs, so I have set their \
visibility to false in KoTextLoader::loadTable, when they are added to QTextDocument.

</pre>
  </td>
 </tr>
</table>





<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>trunk/koffice/libs/kotext/opendocument/KoTextLoader.cpp <span style="color: \
grey">(1139065)</span></li>

 <li>trunk/koffice/plugins/textshape/Layout.cpp <span style="color: \
grey">(1139065)</span></li>

 <li>trunk/koffice/plugins/textshape/TextShape.cpp <span style="color: \
grey">(1139065)</span></li>

</ul>

<p><a href="http://reviewboard.kde.org/r/4366/diff/" style="margin-left: 3em;">View \
Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>



_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic