Bug 135888 - copy/paste of text boxes results in separate shape and text frame
Summary: copy/paste of text boxes results in separate shape and text frame
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.3.0.3 release
Hardware: All All
: medium normal
Assignee: Michael Stahl (allotropia)
URL:
Whiteboard: target:7.1.0 target:7.0.2 target:6.4.7
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-08-18 17:48 UTC by Michael Stahl (allotropia)
Modified: 2020-08-20 12:15 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Stahl (allotropia) 2020-08-18 17:48:40 UTC
in Writer, copy a shape w/ text-box and paste it, the result will be 2 unconnected objects, a shape and a text frame.

the problem is that in DocumentContentOperationsManager::CopyFlyInFlyImpl(),
the

  SwTextBoxHelper::resetLink(pFormat, aOldContent);

removes the RES_CNTNT property that isTextBox() checks to determine it's a text box (it's also required in CopyLayoutFormat()).

   if (!pShape || pShape->Which() != nType || !pShape->GetAttrSet().HasItem(RES_CNTNT))
       return false;

it's only restored at the end of the function, when it's too late:
  
  SwTextBoxHelper::restoreLinks(aSet, aVecSwFrameFormat, aOldTextBoxes, aOldContent);

i don't understand why we need this?

reading the git log it looks to me that this caused it:

commit 0bcc5b3daebeb2a7d2b5ba132af4745cc6c78cd0
Author:     Jan-Marek Glogowski <glogow@fbihome.de>
AuthorDate: Fri Jul 22 17:50:52 2016 +0200

    Switch isTextBox to use the format pointers
    
    This replaces all possible occurences of the text box format
    maps, which just want to know, if a SwFrameFormat is part of a
    text box to use the direct lookup via the isTextBox, which is
    now a cheap call.
Comment 1 Michael Stahl (allotropia) 2020-08-18 18:01:00 UTC
forgot to mention: this happens when you copy text in which a textbox is anchored; if you select the textbox and copy it then it doesn't go via that function and there's a SwUiWriterTest::testFdo82191(); i noticed because i wanted to remove that code but that's futile anyway because it's the only way to copy the horrible at-page flys.
Comment 2 Telesto 2020-08-18 19:33:46 UTC Comment hidden (obsolete)
Comment 3 Miklos Vajna 2020-08-19 07:35:30 UTC
The original doc model I added was that a draw format and a fly format is linked together to form a textbox in case they both have  RES_CNTNT that point to the same content. This has the nice property that those node indexes are automatically updated in many cases, unlike raw pointers.

Jan-Marek noticed some perf problems with this, since in case you have lots of textboxes, then looking up all these pairs is O(N^2) (where N is the number of frame formats in the special frame format array). So he added those raw pointers in the SwFrameFormat class, to have faster way/cache to jump to the "other" frame format (if there is any). Is it possible that at some stage we update RES_CNTNT but not these newer pointers?

The reset/restoreLinks() code is only relevant for linked text frames. I.e. imagine some corner case that you have 2 rounded rectangles and the text content flows from the 1st to the 2nd one. Then you need the 2 draw formats for the rounded corners, but the 2 fly formats will be linked fly frames.

Hope this helps.
Comment 4 Michael Stahl (allotropia) 2020-08-19 09:10:30 UTC
the problem in resetLinks is this:

>694                 pShape->ResetFormatAttr(RES_CNTNT);
Comment 5 Commit Notification 2020-08-19 17:04:50 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/10ae7ba661dff57a7d08174792565ec5e33fae9b

tdf#135412 tdf#135888 sw: fix copying of linked text-boxes

It will be available in 7.1.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 6 Michael Stahl (allotropia) 2020-08-19 17:15:34 UTC
turns out this was the root cause of the other bug i was trying to fix
-> fixed on master
Comment 7 Commit Notification 2020-08-20 00:49:17 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/9ae30972319a046d0b8176e3f0bc85f8ae976c93

tdf#135412 tdf#135888 sw: fix copying of linked text-boxes

It will be available in 7.0.2.

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

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2020-08-20 12:15:39 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/8934e31f53274a5e288340c9f756aac5f931eba4

tdf#135412 tdf#135888 sw: fix copying of linked text-boxes

It will be available in 6.4.7.

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

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