Review Request 120250: Change blockLayout return to be more explicit

[Pierre]

On Wednesday, September 17, 2014 10:16:35 PM Pierre wrote:
> On Wednesday, September 17, 2014 08:09:24 PM Pierre Ducroquet wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://git.reviewboard.kde.org/r/120250/
> > -----------------------------------------------------------
> > 
> > Review request for Calligra.
> > 
> > 
> > Bugs: 306000
> > 
> >     http://bugs.kde.org/show_bug.cgi?id=306000
> > 
> > Repository: calligra
> > 
> > 
> > Description
> > -------
> > 
> > Returns an enum instead of a boolean and relying on an integer value aside.
> > This allows the backtrack code to know that a layout ended because of a page
> > break, and thus not follow the keep with next instead of ending up in an
> > infinite loop.
> > 
> > With that patch, we still have a difference between us and LibreOffice 4.3,
> > a
> > check of the OpenDocument specification will perhaps help : they decide to
> > just skip the page break when it is in a keep with next block.
> 
> FYI, a patch implementing that way of layouting the text is much simpler :
> 
> index 3a1fa57..bd317b1 100644
> --- a/libs/textlayout/KoTextLayoutArea.cpp
> +++ b/libs/textlayout/KoTextLayoutArea.cpp
> @@ -1226,10 +1226,12 @@ bool KoTextLayoutArea::layoutBlock(FrameIterator
> *cursor)
>                  c1.setPosition(c1.position() + 1, QTextCursor::KeepAnchor);
> 
>                  KoTextSoftPageBreak *softPageBreak =
> dynamic_cast<KoTextSoftPageBreak*>(d->documentLayout->inlineTextObjectManager(
> )-
> >inlineTextObject(c1));
> 
> -                bool keepWithNext =
> block.blockFormat().boolProperty(KoParagraphStyle::KeepWithNext);
> -                if (softPageBreak && !keepWithNext) {
> -                    softBreakPos = pos;
> -                    break;
> +                if (softPageBreak) {
> +                    QTextBlock previousBlock = block.previous();
> +                    if (!previousBlock.isValid() ||
> !previousBlock.blockFormat().boolProperty(KoParagraphStyle::KeepWithNext)) {
> +                        softBreakPos = pos;
> +                        break;
> +                    }
>                  }
> 
>                  pos = text.indexOf(QChar::ObjectReplacementCharacter, pos +
> 1);
> 
> 
> I'll try to dig up from OpenDocument specification a recommendation regarding
> the proper way of handling that, if there is any.

I found none neither in the 20.194 fo:keep-with-next section nor in the 
text:soft-page-break section. I don't remember of any other section that may 
contain that information, so I guess it's up to us to choose the way we want to 
implement it… I suppose following the LibreOffice implementation will be better 
from an interoperability point of view…
Can anyone check what decision microsoft took in office regarding that case ?



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140917/20588dda/attachment.sig>