fix: preserve background color for odt

[DmySyz]

Summary

Changed property prioritization for the brush initialization in order to avoid losing properties.

Test

Manually end-to-end.

Notes

Assisted-by: ClaudeCode:claude-sonnet-4-6

[Alex-Arsys]

Hi @DmySyz, upon analyzing it with Claude, we found a case that we might need to take into account.

My concern is that moving shape to the end of the priority list is unconditional, and a DOCX page background with a gradient or picture arrives precisely as a shape. So this trades the ODT bug for a DOCX regression.

Why they collide. In ReadBackground (word/Editor/Serialize2.js), Color, Unifill (ColorTheme) and shape (pptxDrawing) are read from independent tags and can all coexist on the same background. A typical DOCX gradient/picture background is stored as:

<w:background w:color="FFFF00">        <!-- solid fallback  -> Color  -->
  <v:background fillcolor="#00B050">
    <v:fill color2="#FF0000" type="gradient" focus="100%"/>   <!-- real fill -> shape -->
  </v:background>
</w:background>

With this change, when Color and shape coexist the solid color wins and shape.brush (which may be a gradient, picture/blip or pattern) is dropped.

Verified. I built that DOCX (yellow w:color fallback + green→red gradient) and ran an x2t roundtrip docx → .bin → docx. The internal .bin — the same format readFromBinary consumes — preserves both sources: the output document.xml contains w:color="FFFF00" and <v:fill ... colors="0f #00B050;65536f #FF0000;" type="gradient"/>. So the model holds Color and shape together, and after this change the page renders flat yellow instead of the gradient.

Suggested fix — prefer shape only when its brush is usable, otherwise fall back. The ODT bug is specifically a shape whose brush is a solid fill with no color (isVisible() is true, isSolidFill() is false → paints white). We can key off exactly that instead of reordering everything:

DocumentBackground.prototype._getBrush = function()
{
    if (this.shape && this._isShapeBrushUsable())
        return this.shape.brush;

    if (this.Unifill)
        return this.Unifill;

    if (this.Color)
        return AscFormat.CreateSolidFillRGB(this.Color.r, this.Color.g, this.Color.b);

    if (this.shape)
        return this.shape.brush; // last resort: unusable shape brush, no Color/Unifill to fall back to

    return null;
};

DocumentBackground.prototype._isShapeBrushUsable = function()
{
    let brush = this.shape && this.shape.brush;
    if (!brush || !brush.isVisible())
        return false;

    // A solid fill is only usable if it resolved to an actual color; a solid fill with
    // no color is the broken VML case that would otherwise paint the page white.
    if (brush.fill && brush.fill.type === Asc.c_oAscFill.FILL_TYPE_SOLID)
        return brush.isSolidFill();

    // Gradient / picture (blip) / pattern fills carry their own content — use them.
    return true;
};

This keeps your ODT fix (solid-without-color shape → falls back to the reliably-parsed Color/Unifill) while preserving DOCX gradient/picture backgrounds.

[DmySyz]

@Alex-Arsys Thanks for the review! Went with your suggestion.

[Alex-Arsys]

Hi @DmySyz !

Awesome, thanks for applying the suggestion! 🙌 I re-reviewed the whole PR and the logic looks correct: the broken ODT case (solid fill with no color) falls back to Color/Unifill, and DOCX gradient/picture backgrounds are preserved since they aren't SOLID. I also confirmed _getBrush is only used in the render path, so the round-trip stays intact.

Just one minor thing before LGTM: the new block uses spaces for indentation, but the rest of the file uses tabs (it's even mixed on the _isShapeBrushUsable line). Could you re-indent it to tabs to match the file style?

And a quick question on testing: can you confirm you manually checked both cases — the ODT this fixes and a DOCX with a gradient/picture background? That would give me peace of mind. Thanks!

[DmySyz]

Restested and added one more small fix for the brush.

[chrip]

Summary

The priority rework from the earlier round is correct and well-scoped: _isShapeBrushUsable() keys off
exactly the broken ODT case (a SOLID fill with no resolved color), so DOCX gradient/picture backgrounds
still take the shape path. That part I'd approve as-is.

The problem is the "one more small fix for the brush" added in the 2026-07-31 force-push, which nobody
has reviewed yet. Normalizing the shape's solid brush into a fresh fill reads the color before it has
been resolved against the theme, and drops alpha on the way. That regresses theme-colored and
semi-transparent backgrounds — a wider blast radius than the ODT bug being fixed.

Issues

🔴 Blocking — normalization reads the color before check() resolves it

word/Editor/document/document-background.js:71-77

let RGBA = brush.getRGBAColor();
return AscFormat.CreateSolidFillRGB(RGBA.R, RGBA.G, RGBA.B);

getRGBAColor() returns this.fill.color.RGBA (Format.js:6515), but that CUniColor.RGBA starts as
{R:0, G:0, B:0, A:255, needRecalc:true} (Format.js:2428-2435) and is only populated by
CUniColor.check() (Format.js:3142-3149).

Previously _getBrush() returned this.shape.brush itself, so draw():56's brush.check(theme, colorMap)
resolved that object. Now check() runs on the fresh copy, so this.shape.brush is never checked —
and it isn't checked anywhere else either (this.shape.brush appears only in this one place, and the
shape comes straight from oDrawing.content.GraphicObj at Serialize2.js:11591).

Consequences for a background shape whose fill is SOLID:

• theme/scheme color (isSolidFillScheme() is a modeled case, Format.js:6677) — resolution is only
  possible via CSchemeColor.check() with the theme (Format.js:2971), so the read yields the
  unresolved default and the page paints black.
• color Mods (lumMod/shade/tint/alpha) — applied inside CUniColor.check() and lost, since the copy
  carries no Mods and the RGBA it copies is pre-Mods.

Plain sRGB VML fills happen to survive, which is presumably why the manual test passed.

🔴 Blocking — alpha is silently dropped

Same lines. CreateSolidFillRGB(r, g, b) (Format.js:3462) cannot carry alpha;
getRGBAColor() returns an A, and CreateSolidFillRGBA(r, g, b, a) already exists at
Format.js:3466. A semi-transparent solid background renders fully opaque.

Both are fixed by resolving first and using the RGBA constructor — this also re-indents to tabs:

	DocumentBackground.prototype.draw = function(graphics, sectPr, theme, colorMap)
	{
		let brush = this._getBrush(theme, colorMap);
		...
	};
	DocumentBackground.prototype._getBrush = function(theme, colorMap)
	{
		if (this.shape && this._isShapeBrushUsable())
		{
			let brush = this.shape.brush;
			if (brush.isSolidFill())
			{
				brush.check(theme, colorMap);
				let RGBA = brush.getRGBAColor();
				return AscFormat.CreateSolidFillRGBA(RGBA.R, RGBA.G, RGBA.B, RGBA.A);
			}
			return brush; // gradient / blip / pattern — pass through directly
		}

		if (this.Unifill)
			return this.Unifill;

		if (this.Color)
			return AscFormat.CreateSolidFillRGB(this.Color.r, this.Color.g, this.Color.b);

		if (this.shape)
			return this.shape.brush; // last resort: no Color/Unifill to fall back to

		return null;
	};

Alternatively keep _getBrush() pure and move the normalization into draw() after the existing
brush.check() — that avoids widening the signature.

⚠️ Major — the normalization's justification isn't checkable

Normalize to a fresh solid fill to avoid VML-specific internal state that causes CShapeDrawer to
render incorrectly despite correct fillType/RGBA.

"renders incorrectly" doesn't say what was wrong or which document triggered it, and the workaround is
what introduces both blocking issues above. Could you describe the actual symptom and attach the file?
If the real defect is in CShapeDrawer or the VML parser, fixing it there beats copying the brush in the
render path.

⚠️ Major — no test coverage

tests/ has a word/ suite and runAll.js, and _getBrush/_isShapeBrushUsable are pure logic over a
three-field object, so all five branches are cheap to cover: broken-solid shape falls back to
Color/Unifill; gradient shape passes through; Unifill-only; Color-only; unusable shape with nothing to
fall back to. Given this PR has already changed behavior twice in ways manual testing didn't catch,
that's worth the small effort.

ℹ️ Minor

• Unrelated change — window['AscWord']['DocumentBackground'] (line 149) is a closure-compiler
  export fix, unrelated to the background bug, and it's the only bracket-notation export in the file.
  Worth a line in the description or a separate PR; one concern per PR.
• Commit scope — fix: preserve background color for odt has no scope; convention is
  fix(<scope>): …. DCO sign-off is present ✓, but the commit carries no Assisted-by: trailer even
  though the PR body discloses ClaudeCode:claude-sonnet-4-6.

💡 Suggestion

isBrokenSolidFill() is a general CUniFill method named after one specific VML defect. Something like
isSolidFillWithoutColor() describes the state rather than the presumed cause, and won't read as stale
if another parser hits the same shape. Non-blocking.

Verdict

Request changes — the priority rework is right and the earlier feedback was properly applied, but the
newly-added normalization block reads the fill color before check() resolves it and discards alpha,
which turns theme-colored and semi-transparent backgrounds into black/opaque. Fix that (or drop the
normalization pending an explanation of what it works around), re-indent to tabs as previously asked, and
this is good to go.

Assisted-by: ClaudeCode:claude-opus-5

[DmySyz]

@chrip Regarding the major issues:

1. afaik by the time draw() is called the document has been through the layout recalculation which calls check() so the order is correct.
2. There's no alpha in the source format. Background shapes produced by ODT conversion always default to 255.

Regardless of that - implemented the fixes as they don't hurt either.

In addition regarding this

That regresses theme-colored and semi-transparent backgrounds — a wider blast radius than the ODT bug being fixed

Background shapes from ODT conversion are created from a VML <v:background> element, which always uses direct RGB values from fillcolor. Scheme colors in backgrounds come from DOCX, but they use Unifill and not shape - so they never enter this branch.

[chrip]

Re-review: sdkjs PR #61 — fix: preserve background color for odt

Summary

Both blocking issues are fixed and verified. What's left is two mechanical items — the tab
re-indentation that's now been asked for three times, and test coverage. Happy to approve as soon as
those land.

Blocking issues from the last round — both resolved ✓

• Color resolved before it's read — _getBrush(theme, colorMap) now calls
  brush.check(theme, colorMap) before getRGBAColor(), and draw() passes both through. ✓
• Alpha preserved — now CreateSolidFillRGBA(RGBA.R, RGBA.G, RGBA.B, RGBA.A). ✓

On your rebuttal

You're right about scheme colors, and my example was wrong. I checked ReadBackground
(Serialize2.js:11571-11594): the ColorTheme branch routes through CreateThemeUnifill() into
oBackground.Unifill, and only pptxDrawing populates oBackground.shape. So theme colors genuinely
cannot reach the normalization branch, and "regresses theme-colored backgrounds" overstated it. Sorry
for the noise.

On the recalculation ordering I couldn't confirm your point. The background shape doesn't appear to
be part of the recalculated drawing set — it isn't in graphicPages, and the only things that touch it
are draw(), WriteGraphicObj() (Serialize2.js:5326) and getAllRasterImages()
(GraphicObjects.js:4593), none of which resolve the brush. Worth noting CreateUniColorRGB()
(Format.js:3448) doesn't populate CUniColor.RGBA either — that only happens in CUniColor.check().

Either way it's moot now, and the explicit check matches what draw() already does at lines 56-61
(check, then read RGBA), so the code is more obviously correct for the next reader.

One thing worth flagging: the ODT document you retested doesn't exercise the normalization branch
at all — a solid fill with no color fails _isShapeBrushUsable() and falls back to Color/Unifill. So
that test can't validate this path either way. The branch needs a DOCX with a valid solid VML
background (<v:fill> with an explicit color). This is also still Alex's open question from 2026-07-23.

Still open

🔴 Indentation

Verified at c7b6cd2: every line inside _getBrush and _isShapeBrushUsable still uses spaces while
the rest of the file uses tabs, and _isShapeBrushUsable still mixes a tab on its declaration line with
spaces on the opening brace. @Alex-Arsys asked on 2026-07-23

sed -i 's/^ /\t/g; s/^\t /\t\t/g' word/Editor/document/document-background.js gets most of it,
but please eyeball the result. (The green code-style job won't catch this —
tests/code-style/check.py only checks the license header and the file's trailing byte.)

⚠️ No test coverage

Still none. _getBrush/_isShapeBrushUsable are pure logic over a three-field object and tests/word/
already exists, so the five branches are cheap to pin down: broken-solid shape → falls back; gradient
shape → passes through; Unifill-only; Color-only; unusable shape with no fallback. Given the branch your
manual testing can't reach (above), this is the only thing that would actually cover it.

Verdict

Request changes — correctness work is done and verified; this is now just the tab re-indentation away from approval.

Assisted-by: ClaudeCode:claude-opus-5

[moodyjmz]

One more thing worth checking before this lands, separate from the alpha/check() fixes above: _isShapeBrushUsable() treats a genuinely noFill shape brush the same as the broken-VML-solid case.

let brush = this.shape && this.shape.brush;
if (!brush || !brush.isVisible())
    return false;
return !brush.isBrokenSolidFill();

isVisible() is fill.type !== FILL_TYPE_NOFILL (Format.js:6684), so an explicit noFill shape brush fails this check too, and _getBrush falls through to this.Unifill/this.Color.

Before this PR, _getBrush was if (this.shape) brush = this.shape.brush; — unconditional — so a noFill shape brush flowed into draw(), hit if (!brush.isVisible()) return;, and the page rendered nothing.

ReadBackground (Serialize2.js:11571) populates Color, Unifill, and shape from independent tags that can coexist (same mechanism as the gradient-preservation case above). So a document with both a plain Color fallback and a shape whose brush is intentionally noFill would previously render transparent, and will now render the Color fallback instead.

Not sure this is reachable in practice — don't know the VML/DrawingML encoding well enough to say whether real documents actually pair those two things. Flagging as a question: is a legitimately-empty shape brush supposed to fall back to Color/Unifill, or should _isShapeBrushUsable only treat the broken case (isBrokenSolidFill()) as unusable and let a clean noFill brush pass through so draw()'s existing visibility check handles it as before?

[DmySyz]

@moodyjmz makes sense. Changed the behavior.

[chrip]

The correctness work is finished. @moodyjmz's noFill question is implemented correctly, and I
re-verified both of my earlier blocking issues still hold at 2e82b7b — including that alpha genuinely
survives the second check() in draw(), which I hadn't traced before. No remaining objections to the
logic.

Assisted-by: ClaudeCode:claude-opus-5