fix(html): put a frame where its style says, and keep its image inside - #684
Merged
Conversation
A frame's image is absolutely positioned against the frame, but nothing
in the frame's own markup made it a containing block - that came from
`*{position:relative}` in the shipped stylesheet. Read without that
sheet, every image resolved against the initial containing block and
covered the page at full size. The frame now states its own position.
ODF also places a frame by naming a side (`style:horizontal-pos`) rather
than an offset, which was ignored, so a centred image sat hard left.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08cc2008d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d one `style:horizontal-pos="from-left"` names no side, so it read as nothing and left a side inherited from the parent style standing. It now assigns, so the frame's own declaration wins. No rendered output moves - the renderer already ignores a side where an offset places the frame. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT
`GraphicStyle::horizontal_position` stopped at the C++ API; the python, java and objc bindings still ended at `text_wrap`, so no binding consumer could read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT
gcc 14 flags the nullopt branch of the ternary as a maybe-uninitialized read of the optional's payload, which -Werror turns into a build failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVkpzsGS6YASMqm3NwmWib
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
about.odtrendered as a full-page cow: the image covered the whole page instead of sitting in the text.What was wrong
A frame's
<img>is written asposition:absolute;left:0;top:0;width:100%;height:100%, but nothing in the frame's own markup made it a containing block — that came only from*{position:relative}in the shippeddocument.css. Read without that sheet (externalized resources, page moved away from itsresources/dir), the image resolved against the initial containing block: top left of the page, full viewport size. Every image in the document landed there, stacked.Separately, ODF places a frame either by an offset (
svg:x) or by naming a side (style:horizontal-pos). Only the offset was read, so a centred image — the letterhead logo instyle-various-2.odt— sat hard left where LibreOffice centres it.What changed
position:relativeitself, so it is a containing block regardless of the stylesheet.style:horizontal-posis read into a newGraphicStyle::horizontal_positionand applied as auto margins (block frames) orleft:0;right:0(absolute frames), only where nosvg:xalready places the frame.frame.width()->to_string()was an unchecked optional dereference in the wrap-beforebranch while the block below it guards the same optional.Verification
about.odtmeasured as img rect vs. its frame rect, in all four combinations oftext_document_marginand stylesheet:0,0 1200x900, frame at495,977495,977 171x171= frame0,0 1200x900, frame at419,766419,766 171x171= frame690,993690,993— unchanged411,758411,758— unchangedWhere the stylesheet loads, geometry is untouched. Full suite green (861 tests); of all regenerated output, 41 files differ and every one is accounted for by the added
position:relativeand the auto margins — no other rendering moved. Reference output regenerated and the pins advanced.Not addressed here: WMF/EMF images are still inlined as
data:image/wmf, which no browser decodes, so they render as a broken-image icon (style-various-2.odt,journal_Word_template.docx,15-MB-docx-file-download.docx,sample.xlsx). That needs a metafile decoder, like SVM already has.🤖 Generated with Claude Code
https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT