Bug 103975

Summary: FILEOPEN: DOCX: Incorrect page break moves content to page 2
Product: LibreOffice Reporter: Xisco Faulí <xiscofauli>
Component: WriterAssignee: Justin L <jluth>
Status: VERIFIED FIXED    
Severity: normal CC: jluth, raal, stgohi-lobugs, xiscofauli
Priority: medium Keywords: bibisected, bisected, filter:docx, regression
Version: 5.2.2.2 release   
Hardware: All   
OS: All   
See Also: https://bugs.documentfoundation.org/show_bug.cgi?id=104380
https://bugs.documentfoundation.org/show_bug.cgi?id=104710
https://bugs.documentfoundation.org/show_bug.cgi?id=152885
Whiteboard: target:5.4.0 target:5.3.0.1
Crash report or crash signature: Regression By:
Attachments: tdf103975_notPageBreak.docx: reduced test document showing the problem
singleColumnBreak.zip: a suite of test documents

Description Xisco Faulí 2016-11-17 10:07:54 UTC 
Steps:
Open attachment 61292 [details]

Observed behaviour: Content is displayed on the second page
Expected behaviour: Content should be displayed on the first page

Reproduced in 

Version: 5.3.0.0.alpha1+
Build ID: 757a60d01dd152aadab2ba3c8224252481ce8a88
CPU Threads: 4; OS Version: Linux 4.8; UI Render: default; VCL: gtk3; Layout
Engine: new; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

but not in

Version: 4.3.0.0.alpha1+
Build ID: c15927f20d4727c3b8de68497b6949e72f9e6e9e
Comment 1 A (Andy) 2016-11-19 08:14:25 UTC 
Reproducible with LO 5.2.3.3 (Win 8.1)
Comment 2 raal 2016-11-24 06:23:41 UTC Comment hidden (excessive_detail, obsolete)
Comment 3 Xisco Faulí 2016-11-24 09:59:42 UTC 
Yes, I do confirm that reverting 2721111437706372eaac9bf1d748723196c573ac fixes the problem.

Adding Cc: to Justin Luth
Comment 4 Justin L 2016-11-25 06:35:47 UTC 
Created attachment 128999 [details]
tdf103975_notPageBreak.docx: reduced test document showing the problem

So far, my conclusion is that this has uncovered another bug, related to the fact that these columns have no content. Adding a single character in one of the columns prevents the problem. So does removing the HardReturn in the first column. So clearly this is actually in column mode (visually confirmed in the tab bar) even though FindColFrame() finds nothing (section attributes GetCol().GetNumCols() also report 0).

Also - I failed to be able to create a clean-room test document in MSWord to replicate the problem. This minimal unit test is just a reduced version of the original.

The worst case fix for this particular document is to avoid it by:
const bool bPotentiallyHasColumns = m_rThis.FindColFrame() || m_rThis.IsInSct();
Comment 5 Justin L 2016-11-26 10:05:45 UTC 
Created attachment 129021 [details]
singleColumnBreak.zip: a suite of test documents

Wonderful! It wasn't a regression by me.  Fix proposed at https://gerrit.libreoffice.org/31227

There are still a few cases where either the column break is lost (A,C,E), or one where it still causes a page break (D).
Comment 6 Commit Notification 2016-11-26 10:39:21 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7b250d56981f78e77454a2a3fd670731b2358e75

tdf#103975 docx import: don't remove sectPr with BreakItem

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Justin L 2016-11-28 18:03:45 UTC 
proposed fix for lost manual page breaks in tests A,C,E: https://gerrit.libreoffice.org/31326

OK. The round-tripping on OP’s a.docx is horrendous. That probably requires a massive overhaul of handling columns in sections rather than page styles. I don't think I'm going to go there...
Comment 8 Commit Notification 2016-11-29 03:41:38 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bbc7ed9c379019f31dc7ac234cf83ea151601465

tdf#103975 docx import: don't lose column break

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Justin L 2016-11-29 05:31:29 UTC 
proposed fix for test D where the page break still occurred: gerrit.libreoffice.org/31342
Comment 10 Commit Notification 2016-11-29 19:11:01 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d1c06ed189e9af7871961f72acae2dfd938d71d5&h=libreoffice-5-3

tdf#103975 docx import: don't remove sectPr with BreakItem

It will be available in 5.3.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2016-11-30 04:22:52 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4307a850a310c0371bee1f79a8eb41fa8ff7fed2

tdf#103975 docx import: don't lose column break #2

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2016-11-30 05:51:03 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3de0128ec4e0fce998c84043a7f8f173f1f8ac96&h=libreoffice-5-3

tdf#103975 docx import: don't lose column break

It will be available in 5.3.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2016-12-03 04:06:45 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5e74df1c3a09bd00caa4f9b8d3d0cd5539d1acff&h=libreoffice-5-3

tdf#103975 docx import: don't lose column break #2

It will be available in 5.3.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Justin L 2016-12-03 18:02:11 UTC 
round-tripping problem created as new bug 104380.
Comment 15 Justin L 2016-12-20 07:26:44 UTC 
Comment 6 caused regression bug 104710
Comment 16 Commit Notification 2017-05-19 11:34:34 UTC 
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cee02e81cd3b55fb46eacf5db5713e8bf4363bcd

ooxmlexport: roundtrip tdf#103975's unit tests

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.