fix(layout): paint the Word 97-2003 run effects and the double strike - #3983
Open
Nathaniel-260 wants to merge 1 commit into
Open
Nathaniel-260 wants to merge 1 commit into
Nathaniel-260 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Nathaniel-260
force-pushed
the
fix/legacy-run-effects
branch
from
September 6, 2026 23:49
e3d352b to
4512671
Compare
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Nathaniel-260
force-pushed
the
fix/legacy-run-effects
branch
from
September 6, 2026 23:59
4512671 to
00a91f7
Compare
Nathaniel-260
added a commit
to Nathaniel-260/otzaria-word-editor
that referenced
this pull request
Sep 7, 2026
דווח: „כפתור מתקדם פותח פאפ, אני בוחר עיצוב, לוחץ, וזה לא עושה כלום”. נמדד ב-Chrome אמיתי על ה-dist הארוז — פירוק ה-zip של `export.toDocx` מול `getComputedStyle` על `.superdoc-text-run`, ובהשוואת צילום מסך בייט-בבייט של השורה לפני ואחרי — ונמצאו שני דברים שונים לגמרי שנראים למשתמש אותו דבר. **האחד: אין טקסט מסומן.** `format.apply` מקבל SelectionTarget בלבד, ועד עכשיו התשובה הגיעה רק ברגע ה„אישור”: הדיאלוג נסגר, לקח איתו את שבעה-עשר השדות שמולאו, והשאיר „יש לסמן טקסט תחילה” בשורת המצב. זו אינה מגבלת מנוע אלא סדר של ממשק, והיא תוקנה בשלושה חלקים: 1. `hasRangeSelection` (engine/font-advanced.ts) היא אותה קריאה בדיוק של `applyFontAdvanced` — לא שאלה שנייה שעלולה לענות אחרת — והיא נשאלת בפתיחה. ארבע תשובות ולא שתיים: „יש טווח”, „אין בחירה”, „אין `format.apply` בגרסה הזאת”, ו„אין לדעת” שאינו נועל דבר. „אין מסמך” נכלל באחרון: בין מסמכים ובזמן טעינה המסמך הוא `null`, והודעה שאומרת שם „אינו זמין בגרסה זו של המנוע” היא טענה שקרית על הבניין. 2. **והיא נשאלת שוב.** הדיאלוג אינו חוסם את המסמך — אין מאחוריו רקע, הוא נגרר בכוונה, ואפשר ללחוץ בטקסט בזמן שהוא פתוח. תשובה שנקראה פעם אחת מתיישנת בשני הכיוונים, ושניהם מחזירים את הבאג: בחירה שהתכווצה מתחת לכפתור שנשאר פתוח, או כפתור שנעול לנצח אחרי שהמשתמש סימן טקסט כמו שההודעה ביקשה. מאזין `selectionchange` מכסה את שינויי הבחירה, ו-`watch` על המסמך הפעיל מכסה מעבר בין לשוניות — שאינו מייצר `selectionchange`. פס התצוגה המקדימה נקרא מחדש יחד עם התשובה. 3. **והדיאלוג נסגר על הצלחה בלבד.** קודם הוא נסגר לפני שההחלה יצאה לדרך. **והשני: פקדים שנכתבים ואינם מצוירים.** מסגרת לתו, צל, חרוט ושקוע יוצאים ל-docx קנונית ומוצגים ב-Word, והעורך אינו מצייר אף אחד מהם — צילום המסך זהה בייט-בבייט. „קו חוצה כפול” מצויר, אבל כקו בודד. ובאותה רשת פקדים גם „קרנינג” ו„גודל הגופן המורכב” אינם מצוירים, וכך כל מחסנית הכתב המורכב. מבחינת מי שלחץ „אישור” זה בדיוק כמו פקד שאינו עובד — ומכיוון שהפעולה מדווחת הצלחה, גם שורת המצב נשארת ריקה. אין מה לעשות בצד שלנו: `.superdoc-text-run` אינו נושא שום סימן למאפייני הריצה, ולכן אין וו ל-CSS מקומי, והשלב המקדים (docx-preflight) רץ בפתיחה בלבד ולא היה נוגע בעיצוב שמוחל עכשיו — שזה בדיוק המקרה שדווח. מה שכן: הדיאלוג אומר את זה, מתחת לפס התצוגה המקדימה, ורק על מה שנבחר. הכיתוב של הפס הפך ל„כך ייראה ב-Word (קירוב)” — הוא מתחייב על הקובץ, וההודעה שמתחתיו אומרת מה המסך לא יראה. הודעה ולא אזהרה: שום דבר לא נכשל. **התיקון עצמו נשלח למעלה:** superdoc/docx-editor#3983. שתי השכבות שבולעות אותרו במדידה — `dstrike` הוא פער של הצייר בלבד, וארבעת האחרים נבלעים ב-`normalizeRunAttrsFromOoxml`. הגשר אל חוזה הפריסה גנרי, ולכן כל התיקון יושב בקוד ה-OSS. `w:kern` הושאר בחוץ במפורש: קרנינג משנה את רוחב הזחילה של הגליפים, כלומר את המדידה ולא רק את הציור. הרישום המלא ב-docs/engine-gaps.md. **שער חדש:** `npm run check:font-advanced` — 5/5 ב-Chrome אמיתי. שלוש משורותיו מודדות את ההודעה עצמה מול הפיקסלים, פקד אחד לכל ריצה כדי שאפשר יהיה לייחס פיקסל לפקד: ברגע שהמנוע יתחיל לצייר צל, או קו כפול, השורה תיפול ותאמר מה להוריד מההודעה. הודעה שאיש אינו מודד היא הודעה שתשקר בשקט. `check:dialog-drag` נדרש לתיקון: הוא פתח את הדיאלוג על סמן מכווץ, ולכן „Enter מאשר” נמדד על כפתור שנעול מסיבה אחרת. הוא מסמן טקסט לפני, ומפריד עכשיו בין „ה-Enter לא הגיע” לבין „ההחלה נכשלה” לפי שורת המצב. מוטציה, לפני שנסמכים על אחת מהן: כל שומר הוסר בתורו ונמדד שהבדיקה נצבעת אדום — הרשימה, השער, ההודעה, המאזין, מעקב המסמך, ומיפוי „אין מסמך”. שלושים ואחת בדיקות חדשות (רכיב + יחידה). `npm test` מלא: 3992 עוברות; typecheck נקי; שני השערים ירוקים. שמונה בדיקות קיימות שהרכיבו את הדיאלוג בלי בחירה מצהירות עליה עכשיו במפורש — התנאי המוקדם שלו הוא טקסט מסומן. `createSuperdocDouble` קיבל `setSelection`: הבחירה אינה קבועה בזמן ההרכבה, וממשק שקורא אותה יותר מפעם אחת אינו ניתן לבדיקה בלי זה.
`w:outline`, `w:shadow`, `w:emboss` and `w:imprint` are what the Font dialog's "Effects" group writes, and Word still honours them — a document can carry them with no `w14:` counterpart anywhere. They round-tripped correctly through import and export and were then dropped: the normalizer never read them, so nothing downstream could paint them. Applying one to a selection changed the file and left the page identical. `w:dstrike` had the other half of the same problem. `normalizeRunAttrsFromOoxml` already emitted `doubleStrike`, and it reached the painter — the painter simply never read it, so a double strikethrough was drawn as a single line. - style-engine passes the four flags through as authored, including an explicit `false` (`<w:shadow w:val="0"/>` is how Word clears an inherited effect, and dropping the key would let a lower cascade layer switch it on). - `RunMarks` carries them, next to `strike`, for every run kind. - The DOM painter draws them, after `applyTextEffects` so it can see what an authored `w14:` effect already wrote. It stands aside for a `w14:textOutline` that actually painted a stroke, for a `w14:textFill`, and for a `w14:shadow`; it composes with a `w14:glow`, which is a different effect. The outline test reads what was written rather than whether the object exists, because `<w14:textOutline w14:w="0"><w14:noFill/></w14:textOutline>` is a present object that paints nothing. - The outline empties the glyph through `-webkit-text-fill-color`, not `color`. The stroke is `currentColor`, and clearing `color` would take the stroke with it — a run carrying `<w:outline/>` with Automatic color, which is exactly what the Font dialog writes, would have painted nothing at all. Formatting marks opt back out of both inherited properties, so the dot inside an outlined run is not emptied with the text around it. - `doubleStrike` draws its own line: it is a separate mark and a separately settable run attribute, so a run carrying only `w:dstrike` still has a strikethrough. `render-line.ts` re-derives the decoration on the paths that lift an underline onto an overlay, so that rule is now shared rather than written twice. - The five paint-invalidation signatures count them — `textRunMergeSignature`, `hashRunVisualMarks`, `deriveBlockVersion`, `deriveParagraphBlockVersion` and `hashParagraphBlockForTableVersion`. Without those, an edit that flips only `<w:emboss/>` leaves the already-painted span on screen, and two adjacent runs differing only in one flag merge into a single span. Outline is faithful — a hairline stroke with the fill removed is what Word draws. Shadow is a hard-edged offset copy. Emboss and imprint keep the authored glyph color and put a light edge on one side and a dark edge on the other; Word instead paints the glyph near the page color, which needs a resolved page background that paint does not have, and guessing it wrong turns the text invisible. The reasoning sits in the source. CSS carries one `text-decoration-style` for the whole decoration set, so a run that is both underlined and double-struck keeps the underline's authored style and the strike stays single — except on the overlay paths, where the underline has already been lifted off the run and the double can be drawn.
Nathaniel-260
force-pushed
the
fix/legacy-run-effects
branch
from
September 7, 2026 01:01
00a91f7 to
3803f14
Compare
Y-PLONI
pushed a commit
to Nathaniel-260/otzaria-word-editor
that referenced
this pull request
Sep 7, 2026
דווח: „כפתור מתקדם פותח פאפ, אני בוחר עיצוב, לוחץ, וזה לא עושה כלום”. נמדד ב-Chrome אמיתי על ה-dist הארוז — פירוק ה-zip של `export.toDocx` מול `getComputedStyle` על `.superdoc-text-run`, ובהשוואת צילום מסך בייט-בבייט של השורה לפני ואחרי — ונמצאו שני דברים שונים לגמרי שנראים למשתמש אותו דבר. **האחד: אין טקסט מסומן.** `format.apply` מקבל SelectionTarget בלבד, ועד עכשיו התשובה הגיעה רק ברגע ה„אישור”: הדיאלוג נסגר, לקח איתו את שבעה-עשר השדות שמולאו, והשאיר „יש לסמן טקסט תחילה” בשורת המצב. זו אינה מגבלת מנוע אלא סדר של ממשק, והיא תוקנה בשלושה חלקים: 1. `hasRangeSelection` (engine/font-advanced.ts) היא אותה קריאה בדיוק של `applyFontAdvanced` — לא שאלה שנייה שעלולה לענות אחרת — והיא נשאלת בפתיחה. ארבע תשובות ולא שתיים: „יש טווח”, „אין בחירה”, „אין `format.apply` בגרסה הזאת”, ו„אין לדעת” שאינו נועל דבר. „אין מסמך” נכלל באחרון: בין מסמכים ובזמן טעינה המסמך הוא `null`, והודעה שאומרת שם „אינו זמין בגרסה זו של המנוע” היא טענה שקרית על הבניין. 2. **והיא נשאלת שוב.** הדיאלוג אינו חוסם את המסמך — אין מאחוריו רקע, הוא נגרר בכוונה, ואפשר ללחוץ בטקסט בזמן שהוא פתוח. תשובה שנקראה פעם אחת מתיישנת בשני הכיוונים, ושניהם מחזירים את הבאג: בחירה שהתכווצה מתחת לכפתור שנשאר פתוח, או כפתור שנעול לנצח אחרי שהמשתמש סימן טקסט כמו שההודעה ביקשה. מאזין `selectionchange` מכסה את שינויי הבחירה, ו-`watch` על המסמך הפעיל מכסה מעבר בין לשוניות — שאינו מייצר `selectionchange`. פס התצוגה המקדימה נקרא מחדש יחד עם התשובה. 3. **והדיאלוג נסגר על הצלחה בלבד.** קודם הוא נסגר לפני שההחלה יצאה לדרך. **והשני: פקדים שנכתבים ואינם מצוירים.** מסגרת לתו, צל, חרוט ושקוע יוצאים ל-docx קנונית ומוצגים ב-Word, והעורך אינו מצייר אף אחד מהם — צילום המסך זהה בייט-בבייט. „קו חוצה כפול” מצויר, אבל כקו בודד. ובאותה רשת פקדים גם „קרנינג” ו„גודל הגופן המורכב” אינם מצוירים, וכך כל מחסנית הכתב המורכב. מבחינת מי שלחץ „אישור” זה בדיוק כמו פקד שאינו עובד — ומכיוון שהפעולה מדווחת הצלחה, גם שורת המצב נשארת ריקה. אין מה לעשות בצד שלנו: `.superdoc-text-run` אינו נושא שום סימן למאפייני הריצה, ולכן אין וו ל-CSS מקומי, והשלב המקדים (docx-preflight) רץ בפתיחה בלבד ולא היה נוגע בעיצוב שמוחל עכשיו — שזה בדיוק המקרה שדווח. מה שכן: הדיאלוג אומר את זה, מתחת לפס התצוגה המקדימה, ורק על מה שנבחר. הכיתוב של הפס הפך ל„כך ייראה ב-Word (קירוב)” — הוא מתחייב על הקובץ, וההודעה שמתחתיו אומרת מה המסך לא יראה. הודעה ולא אזהרה: שום דבר לא נכשל. **התיקון עצמו נשלח למעלה:** superdoc/docx-editor#3983. שתי השכבות שבולעות אותרו במדידה — `dstrike` הוא פער של הצייר בלבד, וארבעת האחרים נבלעים ב-`normalizeRunAttrsFromOoxml`. הגשר אל חוזה הפריסה גנרי, ולכן כל התיקון יושב בקוד ה-OSS. `w:kern` הושאר בחוץ במפורש: קרנינג משנה את רוחב הזחילה של הגליפים, כלומר את המדידה ולא רק את הציור. הרישום המלא ב-docs/engine-gaps.md. **שער חדש:** `npm run check:font-advanced` — 5/5 ב-Chrome אמיתי. שלוש משורותיו מודדות את ההודעה עצמה מול הפיקסלים, פקד אחד לכל ריצה כדי שאפשר יהיה לייחס פיקסל לפקד: ברגע שהמנוע יתחיל לצייר צל, או קו כפול, השורה תיפול ותאמר מה להוריד מההודעה. הודעה שאיש אינו מודד היא הודעה שתשקר בשקט. `check:dialog-drag` נדרש לתיקון: הוא פתח את הדיאלוג על סמן מכווץ, ולכן „Enter מאשר” נמדד על כפתור שנעול מסיבה אחרת. הוא מסמן טקסט לפני, ומפריד עכשיו בין „ה-Enter לא הגיע” לבין „ההחלה נכשלה” לפי שורת המצב. מוטציה, לפני שנסמכים על אחת מהן: כל שומר הוסר בתורו ונמדד שהבדיקה נצבעת אדום — הרשימה, השער, ההודעה, המאזין, מעקב המסמך, ומיפוי „אין מסמך”. שלושים ואחת בדיקות חדשות (רכיב + יחידה). `npm test` מלא: 3992 עוברות; typecheck נקי; שני השערים ירוקים. שמונה בדיקות קיימות שהרכיבו את הדיאלוג בלי בחירה מצהירות עליה עכשיו במפורש — התנאי המוקדם שלו הוא טקסט מסומן. `createSuperdocDouble` קיבל `setSelection`: הבחירה אינה קבועה בזמן ההרכבה, וממשק שקורא אותה יותר מפעם אחת אינו ניתן לבדיקה בלי זה.
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
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.
The bug
The Font dialog's Effects group in Word writes four run flags —
w:outline,w:shadow,w:emboss,w:imprint— and Word still honours all four. Theypredate the
w14:effect family, so a document can carry them with now14:counterpart anywhere.
SuperDoc imports them, keeps them, and exports them correctly. It never paints
them. Applying one to a selection writes the file and leaves the page identical.
w:dstrikeis the same seam from the other side.normalizeRunAttrsFromOoxmlalready emits
doubleStrikeand it reaches the painter — the painter simplynever reads it, so a double strikethrough is drawn as a single line.
Reproduce
.docxin SuperDoc.format.apply({ target, inline: { outline: true } })(or
shadow/emboss/imprint/dstrike).dstrike, a single line.Or in Word: type a word, Font → Effects → Outline, save, open in SuperDoc.
Measurement
Chrome, packaged dist, one effect per run so each pixel is attributable, with
getComputedStyleon the.superdoc-text-runelement and a byte-compare ofa PNG clip of the line before and after:
document.xmloutline<w:outline/>shadow<w:shadow/>text-shadow, one offset copyemboss<w:emboss/>imprint<w:imprint/>dstrike<w:dstrike/>line-throughsolidline-throughdoubleThe four flags never reached paint at all. Dumping the keys of the
TextRunthepainter receives, on a run carrying each flag, gave:
So
dstrikewas a painter-only gap, and the other four were dropped one layerearlier, in the normalizer. Both halves are in this patch, and the same dump with
the patch applied shows
outline/shadow/emboss/imprinton the runalongside
doubleStrike.The change
style-engine/normalize/run-attrs.tspasses the four flags through asauthored, including an explicit
false—<w:shadow w:val="0"/>is how Wordclears an inherited effect, and dropping the key would let a lower cascade layer
switch it back on. This mirrors the
vanish/specVanishlines directly below.contractscarries them onRunMarks, next tostrike, so every run kindthat supports inline formatting has them.
painters/dom/runs/text-run.tsdraws them in a smallapplyLegacyRunEffects,called after
applyTextEffectsso it can see what an authoredw14:effectalready wrote:
w14:textOutlinethat actuallypainted a stroke, or a
w14:textFill, rather than relying on declarationorder — order would have left its empty fill behind on a run whose
w14:textOutlinehas no fill of its own, turning an authored outline into ahollow glyph. The test is what was written, not whether the object exists:
<w14:textOutline w14:w="0"><w14:noFill/></w14:textOutline>— what Wordwrites for "no outline" — is a present object that paints nothing, and
standing aside for it would leave the run blank;
w14:shadowin the same way, since that is the authoredform of the same drop shadow — while emboss and imprint, which have no
w14:counterpart and are not drop shadows, keep composing;
text-shadowinstead ofassigning over it, because
w14:glowis a different effect from an embossededge and the two compose as two shadow layers — assigning would let a glow
silently erase an emboss on the same run.
The outline empties the glyph through
-webkit-text-fill-color, notcolor.The stroke is
currentColor, andcurrentColorresolves against the element'sown
color; clearingcolortakes the stroke down with the fill, and a runcarrying
<w:outline/>with Automatic color — exactly what the Font dialogwrites — would paint nothing at all.
normalizeRunAttrsFromOoxmldeliberatelyreports Automatic as no color, so that is the default path, not an edge case.
doubleStrikedraws its own line. It is a separateRunMarksfield and aseparately settable run attribute, so a run carrying only
w:dstrikestill has astrikethrough; reading the line off
strikealone would render it undecorated.render-line.tsre-derives the decoration on the two paths that lift an underlineonto a line overlay, so that rule now lives in one shared helper rather than in
three places that had already drifted.
The outline also fixes the formatting marks it would otherwise empty.
-webkit-text-fill-coloris inherited and beats a child's owncolor, so the·and→marks rendered inside an outlined run would have come out as hairlinerings. Both
::afterrules opt back out — they are chrome, not document text.Five signatures gate paint reuse, and all five count the new flags —
textRunMergeSignature,hashRunVisualMarks,deriveBlockVersion,deriveParagraphBlockVersionandhashParagraphBlockForTableVersion. Without them an edit that flips only<w:emboss/>leaves the already-painted span on screen — the same "the file changed and the
page did not" symptom, one layer up — and two adjacent runs differing only in one
flag merge into a single span, painting one run's effect over the other's text.
On fidelity, stated rather than hidden
draws. Stroke weight scales with font size, with a floor so it does not round
away on a low-density screen.
large display sizes is not reproduced.
side and a dark edge on the other; the direction is what separates the two.
Word instead paints the glyph itself near the page color and lets the edges
carry the shape. Doing that in paint needs the resolved page background, which
the painter does not have — and guessing it wrong turns the text invisible.
That is the one place this trades the last of the fidelity for legibility, and
the reasoning is in the source rather than only here.
This is the same trade
applyTextEffectsalready makes forw14:glow, which itapproximates with concentric zero-offset shadows.
One CSS constraint
text-decoration-styleis a single value for the whole decoration set, so a runthat is both underlined and double-struck can keep only one of the two styles.
The underline already claimed it, and it claimed it from an authored value
(
w:u/@w:val) whilew:dstrikehas no style to lose — so the underline keepsits style and the strike stays single. A visible shortfall on a rare combination,
rather than a wrong underline on a common one.
Deliberately not in scope
w:kernis measured as written-but-not-drawn in the same dialog — the paintersets
font-kerning: noneunconditionally and nothing opts a run back in. It isleft out on purpose: kerning changes glyph advance widths, so it is a
measurement change, not a paint-only one, and it belongs with the layout path
rather than in this patch.
Tests
Added, and each was watched fail before it passed — the fix was disabled in place
and the suites re-run:
painters/dom, 18 new cases: the hollow outline; an outlined run with noexplicit color staying visible (the case that goes blank if the fill is
cleared through
color); the single offset shadow; emboss vs imprint differingby side; both scaling with font size; standing aside for
w14:textOutline;standing aside for
w14:textFill; standing aside forw14:shadow; composing withw14:glow; the double strike;a run carrying only
doubleStrike; the underline keeping its style againsta double strike; no effect painting nothing; and the merge signature changing
for each of the five flags. With the painter pass disabled, 5 go red; with the
fill cleared through
coloragain, 2 go red; with the ordering reverted, 1 goesred; with
doubleStrikedropped from the decoration test, 1 goes red.layout-bridge, 2 new cases onhashRunVisualMarks: a different hash foreach flag, and the flags distinguished from one another. Both go red when the
five lines are removed from the hash.
style-engine, 3 new cases: the flags carried through, an explicitfalsepreserved, and absence staying absent. With the four normalizer lines removed,
2 go red.
A note on how this was reviewed: three independent review passes were run over
the change across two rounds. They found eleven real defects, including two that
an earlier revision of this patch introduced — the invisible default-color
outline and the missed invalidation signatures. All are fixed here, each with a
test that was watched go red first.