Fix embedded OLE object IDs in subdocuments - #659
Conversation
yangfan-yf-yf
left a comment
There was a problem hiding this comment.
I reproduced #621 from the public attachment, and the identifier fix itself looks correct. On 9bbfc87b, the seven objects receive distinct paired v:shape/@id and OLE ShapeID values, distinct ObjectID values, and case-normalized unique w14:anchorId values. All seven relationships resolve to unique internal embeddings, and each XLSX passes ZIP, XML, and relationship checks. Existing body collisions, repeated renders, and the documented non-OLE and header/footer boundaries did not reveal another issue.
One fixture prevents me from approving the current head. tests/templates/issue_621_excel_0.xlsx is not the corresponding payload from the public attachment:
- attachment
output/0.tmp_excel_0.xlsx, also stored as the first embedding indemo_doc_with_excel.docx: 4,867 bytes, SHA-25617ce75452390fd3057db51670b37df9c9027654a9589282d0834070ab6496cf9 - committed fixture: 4,887 bytes, SHA-256
3b7d1befbe9fe6d4dfa8bee06b3dc424774b37079a884e89427c78577516805f
The workbook content is equivalent, but docProps/app.xml and docProps/core.xml differ, showing that this fixture was regenerated. The other six XLSX files and both DOCX templates are byte-identical to the attachment. This makes the “seven exact XLSX payloads” claim and the first expected hash inaccurate.
Please replace issue_621_excel_0.xlsx with the attachment’s exact payload and update its expected size and SHA-256. I did not find another blocker in the implementation.
|
Thanks for tracing the first workbook back to the public attachment. The
I re-downloaded and froze the original attachment tar (125,534 bytes, SHA-256 No production OLE logic or other fixture changed in this follow-up. |
ahmetmusab42-stack
left a comment
There was a problem hiding this comment.
I rechecked exact head abdd10c after the requested fixture correction. I downloaded the original #621 tar attachment again (125,534 bytes; SHA-256 e8dc9d28…5d6c78) and independently confirmed that issue_621_excel_0.xlsx is now byte-identical to the requested source member: 4,867 bytes, SHA-256 17ce7545…6cf9.
The public reproduction passes with seven distinct OLE/shape/ObjectID/anchor/relationship/embedding targets and sheets No.1 through No.7. The destination-collision, repeated-subdocument, and non-OLE VML boundary regressions also pass. I also ran Flake8 and built the sdist and wheel successfully.
I did not repeat the interactive Word/Excel activation roundtrip, so I am not claiming that GUI check, but I found no remaining blocker in the package/XML behavior or in the requested fixture correction. LGTM.
Why
When several subdocuments contain embedded Excel objects, the generated DOCX
can retain all icons and packages while Word exposes only the first Worksheet
Object. Users see seven objects but cannot open six of them.
Root cause
SubdocComposercopied related parts without normalizing VML shape IDs, OLEObjectIDs, and paragraph anchor IDs that must be unique in the destination
document.
Change
shape/@idand OLEShapeIDvalues togetherObjectIDand case-normalizedw14:anchorIdvaluespayload and bind all nine fixture mappings to the frozen attachment
sys.executable, propagate child failures, and emit a deterministicSHA-256 RUN/PASS ledger
Validation
fixtures byte-for-byte; the corrected first workbook is 4,867 bytes with
SHA-256
17ce75452390fd3057db51670b37df9c9027654a9589282d0834070ab6496cf9targets, VML/OLE IDs, ObjectIDs, anchors, valid OOXML packages, and sheets
No.1throughNo.7checks, repository-native suite, strict per-script suite, flake8,
wheel/sdist builds, and archive audits in both review phases
the document, and confirmed all seven still map to
No.1throughNo.7Scope and risks
This remains limited to body-level OLE objects using the sibling VML layout in
the reported attachment. It does not generalize arbitrary non-OLE VML ID
deduplication or OLE objects in headers, footers, or footnotes. The launcher
change ensures this PR's regressions are real CI-discovered tests; broader
nested-command cleanup remains separate. Remote GitHub Actions still require
repository-side approval to run.
Closes #621