Bug 162304 - FILEOPEN ODT: IsFollowingTextFlow is inconsistent regarding "top" and "from top" of "Page text area"
Summary: FILEOPEN ODT: IsFollowingTextFlow is inconsistent regarding "top" and "from t...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: low minor
Assignee: Justin L
URL:
Whiteboard: target:25.2.0
Keywords: implementationError
Depends on:
Blocks: layoutInCell 160077
  Show dependency treegraph
 
Reported: 2024-08-01 20:00 UTC by Justin L
Modified: 2024-08-24 19:40 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
layoutInCell page top ODT.odt: image FollowTextFlow at top of page text area (63.65 KB, application/vnd.oasis.opendocument.text)
2024-08-01 20:00 UTC, Justin L
Details
layoutInCell page top ODT.pdf: how it looks in 25.2 master (57.51 KB, application/pdf)
2024-08-01 20:03 UTC, Justin L
Details
layoutInCell page from top ODT.odt: same document, but "from top" by 0cm (63.00 KB, application/vnd.oasis.opendocument.text)
2024-08-01 20:07 UTC, Justin L
Details
layoutInCell page from top ODT.pdf: how it looks in 25.2 master (57.52 KB, application/pdf)
2024-08-01 20:14 UTC, Justin L
Details
layoutInCell page left.odt: entire page (cell edge), and page text area (at padding) LEFT (155.61 KB, application/vnd.oasis.opendocument.text)
2024-08-02 01:28 UTC, Justin L
Details
layoutInCell page bottom.docx: shows MSO complete failure (149.91 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2024-08-02 14:05 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2024-08-01 20:00:38 UTC
Created attachment 195647 [details]
layoutInCell page top ODT.odt: image FollowTextFlow at top of page text area

ODF 1.3: "The style:flow-with-text attribute specifies whether a drawing shape flows with the text of its layout environment or not. The layout environment of a drawing shape is determined by the location of its anchor."

So what is supposed to happen when the shape has a relation orientation to the page? I would assume that it should attempt to go that specified position on the page, and if that is outside of the layout environment, it will end up at the edge of the layout environment.

Or more concretely, if an image in a table cell is set to go to the top of the page, it will end up at the top of the cell instead, since that is as close as it can come to its desired position.

P.S. Microsoft is not much help to us here. Their implementation is completely inconsistent, handling vertical and horizontal differently. AFAICS, for vertical the offset is applied to the cell text area (the print area - or "margin").

In our current implementation, "top" is connected to the cell's "print area" and "from top" seems to refer to edge of the cell (but might be buggy/based on invalidated print area).

To see what is happening in the example document, select the entire cell, and then go to table properties. That will show you the border spacing for that specific cell. In this case cell A1 has 2cm top and bottom padding and cell B1 has 2.25cm padding (which is what becomes the effective margin for the entire row).

I would have expected the image to be at the edge of the cell, not stuck below the cell padding.

[A follow-up attachment will show that "from top" offset by 0cm is at the cell edge, so our implementation here is inconsistent.]
Comment 1 Justin L 2024-08-01 20:03:56 UTC
Created attachment 195648 [details]
layoutInCell page top ODT.pdf: how it looks in 25.2 master
Comment 2 Justin L 2024-08-01 20:07:56 UTC
Created attachment 195649 [details]
layoutInCell page from top ODT.odt: same document, but "from top" by 0cm

I would have expected these two documents to look identical. I would suggest that this one is laid out properly. The properties of the cell should not affect the positioning of a "page offset".
Comment 3 Justin L 2024-08-01 20:14:45 UTC
Created attachment 195650 [details]
layoutInCell page from top ODT.pdf: how it looks in 25.2 master

The code for this is SwToContentAnchoredObjectPosition::CalcPosition()

For "top", the position comes from GetVertAlignmentValues, while "from top" just uses the cell's print area (rPageAlignLayFrame.getFrameArea()).
Comment 4 Justin L 2024-08-02 01:28:45 UTC
Created attachment 195655 [details]
layoutInCell page left.odt: entire page (cell edge), and page text area (at padding) LEFT

This matches MSO's horizontal.

In LO, the UI doesn't allow "from left" 0cm (and acts a big crazy for "page text area" - forcing it to the far right).
Comment 5 Miklos Vajna 2024-08-02 06:53:12 UTC
Brief feedback if that helps: I think that IsFollowingTextFlow is meant to be a 1:1 mirror of what layoutInCell does in Word. So if there is a matching positioning setting (hori/vert pos/alignment/relation) in Word, we should probably behave exactly they way Word does, if it's silly. For cases that Word does not have, of course behaving what feels correct is a good idea.
Comment 6 Justin L 2024-08-02 12:17:57 UTC
(In reply to Miklos Vajna from comment #5)
> Brief feedback if that helps: I think that IsFollowingTextFlow is meant to
> be a 1:1 mirror of what layoutInCell does in Word... [even] if it is silly.
That is an idea that bites both ways. It becomes a documentation problem, and it also becomes a problem when at some point Microsoft fixes their buggy stuff, and we are left with a documented base implementation that is really lousy.

But yes, I totally see the logic in "not going our own way" too.

I think a critical piece of information here is that "page" should no longer be thought of literally as "page" but as some nebulously termed "current environment frame", so that ENV_FRAME <> PAGE_FRAME and ENV_PRINT_AREA <> PAGE_PRINT_AREA.

For this specific bug report then, the ideal solution would be for the UI to only allow "top" and "from top" and dis-allow vertical "entire page" when following text flow. That would match MSO AFAICS, and still allow for a base implementation to properly handle the other logical choices.
Comment 7 Justin L 2024-08-02 12:33:45 UTC
(In reply to Justin L from comment #0)
> So what is supposed to happen when the shape has a relation orientation to
> the page? I would assume that it should attempt to go that specified
> position on the page, and if that is outside of the layout environment...
This dilemma is solved by realizing that "page" does not mean "page". The two choices mean "at constraining container's edge" and "after the constraining container's spacing".

> I would have expected the image to be at the edge of the cell, not stuck
> below the cell padding.
Thus, this example makes sense, and is working as expected. The image is oriented to "Page text area" (aka PAGE_PRINT_AREA), and since "page" in this case means "cell", "top" starts at the edge of the (effective) cell border spacing.

That means that comment 2's example is the buggy one.
Comment 8 Justin L 2024-08-02 14:05:05 UTC
Created attachment 195667 [details]
layoutInCell page bottom.docx: shows MSO complete failure

As mentioned - MSO fails matching vertical "page" top to the cell top, but instead matches "margin top". In fact, EVERY choice matches margin top (including centre and bottom).  Margin top is fine.
Comment 9 Commit Notification 2024-08-17 21:06:29 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e6a5fa797333ebc526a5ff75e8ec866f3e1cf95c

tdf#162304 layoutInCell: native ODT handling from top of PRINT_AREA

It will be available in 25.2.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 10 Justin L 2024-08-17 22:24:51 UTC
There was the idea in here that we could limit the UI to only allow top/fromTop for vertical page. However, that looks incredibly complicated.

I'll just mark this bug as fixed.