Bug Hunting Session
Bug 89482 - DOC import: broken cross reference field
Summary: DOC import: broken cross reference field
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.5.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:4.5.0 target:4.3.7 target:4.4.2
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2015-02-19 19:27 UTC by Andras Timar
Modified: 2015-12-17 08:46 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
bugdoc (45.00 KB, application/msword)
2015-02-19 19:27 UTC, Andras Timar
Details
Broken cross-ref .doc example (24.00 KB, application/msword)
2015-03-02 15:46 UTC, Justin L
Details
original odt file (13.38 KB, application/vnd.oasis.opendocument.text)
2015-04-06 08:45 UTC, Orwel
Details
file saved as docx (8.14 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2015-04-06 08:46 UTC, Orwel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Timar 2015-02-19 19:27:46 UTC
Created attachment 113522 [details]
bugdoc

The attached document has a cross reference field, which is broken. It shows "Error: Reference source not found" instead of "1.1".
Comment 1 Andras Timar 2015-02-19 19:31:12 UTC
It is a regression from aa852c88c8312512e3595accefb9bc4d234a26a3

fdo#68963 recognize __RefHeading__ bookmarks as TOC in .doc files
Comment 2 Matthew Francis 2015-02-20 01:52:10 UTC
If you've already gone to the trouble of identifying the commit, I think this can be -> NEW :)
Comment 3 Commit Notification 2015-02-28 14:24:46 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

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

tdf#89482 - return size of system virtual device resource at construction.

It will be available in 4.5.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 4 Andras Timar 2015-03-01 18:34:56 UTC
(In reply to Commit Notification from comment #3)
> Michael Meeks committed a patch related to this issue.

Wrong bug id in commit message, it is unrelated.
Comment 5 Justin L 2015-03-02 15:46:17 UTC
Created attachment 113826 [details]
Broken cross-ref .doc example

Replaces the original example which was saved with the "corrupt" cross-reference, and so displayed the "Reference source not found" error in EVERY version of LO.
Comment 6 Justin L 2015-03-02 18:30:19 UTC
The regression is caused by sw/source/filter/ww8/ww8par5.cxx line:     pReffedStck->NewAttr( aStart, SwFltBookmark( EnsureTOCBookmarkName( sOrigName ), aVal, pB->GetHandle(), IsTOCBookmarkName( sOrigName ) ));

The reference name is "__RefHeading__3_20764935".  Previously the call to IsTOCBookmarkName was returning false - now it is returning true.
Comment 7 Justin L 2015-03-03 20:42:02 UTC
I am pretty sure that _Toc is an MSWord-only convention, and that __Refheader__ is a comparable LO convention.  So after a long review, I think my original "fix" is still conceptually valid.  (see CrossRefBookmark.cxx, as well as all of the "ensure __Refheader__ prefix whenever isTocBookmark is true).

Not all references to __Refheader__ bookmarks update aReferencedTOCBookmarks, and ww8par.cxx deletes any TOC bookmarks that aren't there (and thus this regression).  Since ww8par.cxx is primarily concerned about importing Word's .doc format, I think it is safe to change it so that ONLY "_Toc" bookmarks that are not referenced are deleted.  That would leave all LibreOffice generated bookmarks intact.

However, this doesn't resolve another complication: Only ONE crossreferenceBookmark is allowed - so that might cause other regressions if more than one bookmark attempts to reference the same header.  (sw/source/core/doc/docbm.cxx:383: MarkManager::makeMark(..) - refusing to create duplicate CrossRefBookmark)

Alternative 1)   Reverse bug 68963 and find out why a REFERENCE *must* be marked as a TOC in order to be visible.  (I haven't been able to discover why). This would be the ideal alternative if possible.

Alternative 2)   In the CrossRefHeadingBookmark constructor, append a "_Toc" to the prefix.  This might work without causing regressions, but it would affect all of LO, not just .doc.  This would only affect newly created documents.
Comment 8 Andras Timar 2015-03-03 21:09:30 UTC
(In reply to Justin L from comment #5)
> Created attachment 113826 [details]
> Broken cross-ref .doc example
> 
> Replaces the original example which was saved with the "corrupt"
> cross-reference, and so displayed the "Reference source not found" error in
> EVERY version of LO.

It was saved from Word 2010, and it works in Word 2010. I appreciate, if it is a different problem. Your bugdoc is the one, that I wanted to create really.
Comment 9 Commit Notification 2015-03-05 09:56:21 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fe00fba292a9ad1c416acc199e7aba6657ab40b3&h=libreoffice-4-4

tdf#89482 - return size of system virtual device resource at construction.

It will be available in 4.4.2.

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 10 Andras Timar 2015-03-05 10:38:33 UTC
(In reply to Commit Notification from comment #9)
> Michael Meeks committed a patch related to this issue.
Again, it is for bug 89492.
Comment 11 Justin L 2015-03-10 07:56:38 UTC
fix proposed at https://gerrit.libreoffice.org/#/c/14822/

Notes:
1.)  The major part of the fix is in Read_F_Ref - adding a common block of code to ensure that aReferencedTOCBookmarks is updated.

2.)  There were still combinations that would cause "duplicate" cross-references to be attempted, so I added _Toc to all cross-ref bookmarks. (docbm.cxx: SAL_WARN("sw.core", "MarkManager::makeMark(..) - refusing to create duplicate CrossRefBookmark")  These combinations exist in the updated .doc testfile included in the patch.

3.) The "_Toc" additon will affect all formats, not just .doc.  I couldn't find any other way to identify a REF_CONTENT type of .doc bookmark though, and don't think this should cause any regressions.  (code search for __Refheading__ |
GetCrossRefHeadingBookmarkNamePrefix() | isTOKBookmark | 
IDocumentMarkAccess::CROSSREF_HEADING_BOOKMARK).  Just to be safe, I won't push the crossreference.cxx change to the stable 4.3 branch, meaning that although it will be able to edit existing 4.4.2 authored docs OK, it can't create an original doc with working crossref headers (which it already can't do).

Sorry for the initial regression.  I hope this patch is better.
Comment 12 Commit Notification 2015-03-14 10:46:15 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#89482 fix __refheading__ regression, set only CrossRefs as TOC.

It will be available in 4.5.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 13 Commit Notification 2015-03-19 11:02:41 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

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

tdf#68963 avoid __refheading__ regression reported in tdf#89482

It will be available in 4.3.7.

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 Commit Notification 2015-03-19 11:02:48 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=92a43d05d14b1ab7b8bfb4052874d0f0e6c62ef9&h=libreoffice-4-4

tdf#89482 fix __refheading__ regression, set only CrossRefs as TOC.

It will be available in 4.4.3.

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 15 Commit Notification 2015-03-20 18:57:26 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-4-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18b9bb8bafc3946305e2b011f5f100cb34fea959&h=libreoffice-4-4-2

tdf#89482 fix __refheading__ regression, set only CrossRefs as TOC.

It will be available in 4.4.2.

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 16 Orwel 2015-04-06 08:44:42 UTC
I think the problem is solved in 4.4.2 with a .doc file. But if you save a cross reference to a .docx file and try to open the docx file, LO crashes. I attach 2 files (original odt + file saved as docx which cause LO crash).
Comment 17 Orwel 2015-04-06 08:45:41 UTC
Created attachment 114641 [details]
original odt file
Comment 18 Orwel 2015-04-06 08:46:31 UTC
Created attachment 114642 [details]
file saved as docx

if I try to open this file in LO, LO crashes
Comment 19 Orwel 2015-04-06 19:46:40 UTC
Test environment: Windows 7 (64bit).
LibreOffice 4.4.1 failed .doc file as described in this bug.  LibreOffice 4.4.2 opens .doc file correctly and without crashing.
LibreOffice 4.4.1 failed .docx file as described in this bug

LibreOffice 4.4.2 crashes when opening .docx (if quickstart not running, all other LO closed). 

Returning status to "fixed" since the .doc file works properly.  A new bug will be created for the unrelated .docx crashing problem.
Comment 20 Robinson Tryon (qubit) 2015-12-17 08:46:55 UTC Comment hidden (obsolete)