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

List:       koffice-devel
Subject:    Re: Review Request: Small KWord fix for continuous pages adding
From:       "Miroslav Nohaj" <miroslav.nohaj () ixonos ! com>
Date:       2010-06-03 6:29:54
Message-ID: 20100603062954.26427.17884 () localhost
[Download RAW message or body]



> On 2010-06-02 06:52:10, Miroslav Nohaj wrote:
> > Could anybody please take a look at it? It would be great if Thomas could...
> 
> Thomas Zander wrote:
> Didn't you say the fix isn't correct? Looking casually, it certainly is not \
> correct. I don't have time to tell you how to fix the issue if you don't put up a \
> lot more information and do the research needed to find out what needs to be done. 
> Miroslav Nohaj wrote:
> No, I said that 'this might not the best solution' :) But it still works and solves \
> the issue. I believe that this is the right place for the fix from what I've read \
> and understood from the code. I just need some simple advice from you (as it is \
> mostly your work) what is a better place for that fix - like 'move it to a \
> different class' or 'handle that size at a different place - in XXXX at YYY' to \
> make you (as the maintainer / architect of the layout) satisfied with the fix. Just \
> saying 'is not correct' won't help KWord and me neither :) 
> Thomas Zander wrote:
> > I believe that this is the right place for the fix from what I've read and \
> > understood from the code. 
> 
> the first line checks for the document having only one paragraph, that may fix \
> *your* one specific case but its not a really good solution that fixes the real \
> issue.  We can't have fixes like that in KWord if it is to stay maintainable.

> the first line checks for the document having only one paragraph, that may fix \
> *your* one specific case but its not a really good solution that fixes the real \
> issue

This is something that comes from the part of the code I'm trying to fix - the \
original code has that 'if document()->blockCount() == 1' on line 554 (with the diff \
it's on line 561) - in the loop which I added it's not really necessary, but it just \
avoids that loop in cases that the calculated value from the loop would be unused \
(because of line 554 / 561). The problem happens on that block code bellow that 'if' \
- if that's not the real issue which needs to be fixed.... :) 

So, do you have any real suggestion what to do instead?


- Miroslav


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


On 2010-05-28 16:23:29, Miroslav Nohaj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4174/
> -----------------------------------------------------------
> 
> (Updated 2010-05-28 16:23:29)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> When the .odt document contains only one paragraph which contains something (i.e. \
> image) that is bigger than one page, the layout tries to add one page to make it \
> fit, but as that larger thing won't be put on the first page, the situation repeats \
> and it tries to add another page, and so on... 
> The patch checks if the next shapes (together) have enough space to hold the \
> current large thing and if they do, it will not try to add more pages. I know that \
> this might not the best solution so I'm awaiting some recommendations from Thomas \
> on what would be a better way to do it and as this patch will be probably remade, \
> it doesn't contain comments in the code.  
> 
> Diffs
> -----
> 
> /trunk/koffice/kword/part/frames/KWTextDocumentLayout.cpp 1131075 
> 
> Diff: http://reviewboard.kde.org/r/4174/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Miroslav
> 
> 

_______________________________________________
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]