From 2cdffe26a37fd1b403986c86c860b0677f46c5ff Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 11 May 2026 11:26:03 -0300 Subject: [PATCH 1/5] fix(converter): preserve theme rFonts on DOCX round-trip (SD-2894) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cascade's font-slot dedup only handled the ascii/asciiTheme pair, so a higher-priority style with hAnsiTheme/eastAsiaTheme/cstheme would still leave the lower-priority concrete name in place. Word reads the concrete value as an override that defeats per-script theme resolution — Latin body text mapped to majorBidi rendered as Calibri Light instead of the per-script font (Times New Roman for Hebrew/Arab). Extend the dedup to all four slots, and add the same logic to calculateInlineRunPropertiesPlugin so theme refs from the imported run survive the mark-driven recompute that runs on initial load. Verified with a round-trip integration test on the customer fixture: asciiTheme count 65 → 65 (was dropping to ~25 before the fix), no concrete font substitutions on runs that originally carried theme references. --- .../layout-engine/style-engine/src/cascade.ts | 45 +++++++-- .../run/calculateInlineRunPropertiesPlugin.js | 36 ++++++- .../v1/tests/data/sd-2894-theme-rfonts.docx | Bin 0 -> 17173 bytes .../sd-2894-theme-rfonts-roundtrip.test.js | 92 ++++++++++++++++++ 4 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx create mode 100644 packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index 505db8d310..daa0e68543 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -104,20 +104,49 @@ function isObject(item: unknown): item is PropertyObject { * @param propertiesArray - Ordered list of run property objects. * @returns Combined run property object. */ +// SD-2894: `` exposes four independent script slots (ascii / hAnsi / eastAsia / cs). +// Each slot can be filled either by a concrete font name (`w:ascii="Calibri"`) or a theme +// reference (`w:asciiTheme="majorBidi"`). When a higher-priority style supplies one form for a +// slot, the other form from lower-priority sources is no longer applicable and must be +// dropped — otherwise the merged run carries both, and Word resolves the concrete name as an +// override that defeats per-script theme resolution (Latin body text mapped to `majorBidi` +// then renders as Calibri Light instead of Times New Roman for Hebrew/Arab). +// +// Pair `` with `Theme`, except for the cs slot whose theme attribute is the +// lowercase `cstheme` (OOXML inconsistency, not a typo). +const FONT_SLOT_THEME_PAIRS: Array<[keyof RunFontFamilyProperties, keyof RunFontFamilyProperties]> = [ + ['ascii', 'asciiTheme'], + ['hAnsi', 'hAnsiTheme'], + ['eastAsia', 'eastAsiaTheme'], + ['cs', 'cstheme'], +]; + +function dropConflictingFontSlots( + target: RunFontFamilyProperties, + source: RunFontFamilyProperties, +): RunFontFamilyProperties { + const result = { ...target }; + for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) { + if (source[themeKey] != null) { + delete result[concreteKey]; + delete result[themeKey]; + } else if (source[concreteKey] != null) { + delete result[themeKey]; + } + } + return result; +} + export function combineRunProperties(propertiesArray: RunProperties[]): RunProperties { return combineProperties(propertiesArray, { fullOverrideProps: ['color'], specialHandling: { fontFamily: (target: Record, source: Record): unknown => { const fontFamilySource = { ...(source.fontFamily as object) } as RunFontFamilyProperties; - const fontFamilyTarget = { ...(target.fontFamily as object) } as RunFontFamilyProperties; - if (fontFamilySource.asciiTheme != null) { - delete fontFamilyTarget.ascii; - delete fontFamilyTarget.asciiTheme; - } - if (fontFamilySource.ascii != null) { - delete fontFamilyTarget.asciiTheme; - } + const fontFamilyTarget = dropConflictingFontSlots( + (target.fontFamily as RunFontFamilyProperties) ?? {}, + fontFamilySource, + ); return { ...(fontFamilyTarget as object), ...(fontFamilySource as object) }; }, }, diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js index f33a09ac31..4d5eb1952a 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -28,6 +28,40 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([ export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']); +// SD-2894: when the plugin overrides a run's fontFamily from mark-derived values, the +// mark only carries the resolved CSS font name (`ascii: "Calibri Light"`) and forgets the +// theme reference (`asciiTheme: "majorBidi"`) that came from the imported ``. +// Without this merge we'd export concrete font names that defeat Word's per-script theme +// resolution. The pairs match the four `` slots — note `cstheme` is lowercase +// (OOXML inconsistency). +const FONT_FAMILY_THEME_PAIRS = [ + ['ascii', 'asciiTheme'], + ['hAnsi', 'hAnsiTheme'], + ['eastAsia', 'eastAsiaTheme'], + ['cs', 'cstheme'], +]; + +/** + * Merge mark-derived concrete fontFamily slots with theme references preserved on the + * existing run. For each slot where the existing fontFamily has a theme reference, keep + * the theme and drop the concrete value from marks; otherwise take the mark value. + * + * @param {Record|null|undefined} fromMarks + * @param {Record|null|undefined} existing + * @returns {Record} + */ +function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) { + const merged = { ...(fromMarks || {}) }; + if (!existing || typeof existing !== 'object') return merged; + for (const [concreteKey, themeKey] of FONT_FAMILY_THEME_PAIRS) { + if (existing[themeKey] != null) { + merged[themeKey] = existing[themeKey]; + delete merged[concreteKey]; + } + } + return merged; +} + const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys'; const COMPANION_INLINE_KEYS = { fontSizeCs: 'fontSize', @@ -539,7 +573,7 @@ function getInlineRunProperties( const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, editor.converter?.convertedXml ?? {})[0]; const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, editor.converter?.convertedXml ?? {})[0]; if (JSON.stringify(markFromMarks?.attrs) !== JSON.stringify(markFromStyles?.attrs)) { - inlineRunProperties[key] = valueFromMarks; + inlineRunProperties[key] = mergeFontFamilyPreservingThemeRefs(valueFromMarks, existingRunProperties?.[key]); } } else { inlineRunProperties[key] = valueFromMarks; diff --git a/packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx b/packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx new file mode 100644 index 0000000000000000000000000000000000000000..49ce9f63a141b3cf7b09b50a976673ffcc64ea6e GIT binary patch literal 17173 zcmaib1ymhPwlxHI4-zy$(BSUw?(PJ4cXtWy?(XjH7BslKyTea1GcR9w^RIW;s)p{{ zd*A9h)u(nf8sc2gnypMt#d4jvm;lLQX1P(C$Gdm*)i4eoy&K^;t7d}+;3Rfud1PTmm5 zW8ZaKB?`#FN~0_tOvC$ftUd_JpRT`_qqrb0YR}m?CaD}J#Y7nDAXSAzV9&wAh9;o^ zC?jATFT%hJZID953`8cg4#ZR>$D!g?6Do`C>xwA5CiwgU5x;`RG*Wq^mAy_Vr(;gl ztTK5>FAN;5WM64sQZ4d@he`!9hsDGGKvv`>Nh+dhUziq4<~s)3YJ!wv8?58Z@}yyw zLPQvU&nNA|5K31$H7Dl!9P+wF)3q!4T*8`%g|B>+c!Wuf^!%!oJk1)YV3R%C&zMIe zlRs?*zHkvt*SwatGdtj~4Ru%m(6yzWLpioQ*5bKRodOpUJAB55Il{@VT><63u)4zg z5f;v=<=9+R%aVTufN*_tUmv8`dYPB`fmupr$z=_`J#oEfKwg?huVlEW!n)TL;$VvD+5l`?r`p0SBNEJe#T;N*u3o#!5WVRMmZ_^2V|c+x%oE-al% zc2yBSxT6d4!)(IA{-4^o&pcS0MdTJ#FW

wTns%am`0j3U=V{kn1cKxi4GiScfIT z*}Pg%?fpkT_blP)4@?#K>-5Wboj*{=BgugC3jkp=s-&7<-PRZ?4@AHl<2bGC;!_jV zuqz!;=m-8U!;s)Gn-ve1l7Nm?9yJA`bK|OZU_3ifzt6}N58!F>KQHo0ED*& zaUTQ$-&`VO^-;Ii2d6AV(&&U@nB^ zGv&Y=r5G8y91$Z=V8fV$;t;Hem6AiO9jhDF8&NfE`niwlQ7s1t%@UFXdmVhrz#rlY z6m(LeX!?ch9%Yuo@^~So&ih;qY2^*mPMu64D~eK zn3hQACX2n&g!jXNh(9}$Fm+~ikPzY?O$mY$!Nyrvgz2G%Vi^e1LAf*lCxB|SD-5Yd zFnFPNs*^X@VDjaCvd1kg8$7F%sh|Lygu3??h^?~~BkTQ;CA2xz`S=zga!1$FHbJm* zqb-ubacE_7Z0MfM3>j@iT*c}>;Z7{7NM2eX?U+N#_QhCDv?me}_+$qP_v8h1u&EJ$ z#g(JzgX7t*<30nLJ|`zk#k+jH^oE88l9bGaj|fEQrxbIOk34@`Q<8rsE04`PpIX!|0_O zjMhdF4}h>EAO&6A*zmOhR*PF$a*K)y!im$7MqgbtgY*esOCK(RKhCp~Zm4Lo(r50^Z7Z}hf4 z(Jb_;o!bmM>v>$B;}NQOeu~HEX;zYPcWJg0GT0zx0TEzU0b8I`Y0pyPDn>(Tt>=2M zQ)SE8xk<4_mSy~zul$#4pUA%>7XZzjA)7em)Riz@7{MH%QPf9j(sPzm@-oIk;ROkO zp(5orV=*uSMt4D1#B`HTE@tDGwiTRzsLqSxHL?Jwg*J*bBglyN4M4F-)wx%Wlv@8X z8lvPma`6EgBn2D+GZK3DSsc7C(o|G28Q8|-IVRCxC#XsbRumx`f^(#xt>8DH(9Bb zsEBkhDhLC`c6mh^_6q*>#)2o?u5jq=D0x7xVqi52268*tA2m73`Ntp8=i~|I;#%Xy zJv1v5^F)diRT4mFEWVnYJ$hTxs{2b;AQZM$ZrkT+H^CZKw&pz6otn&wnme#ql?L76 z6LCg*PJBh!_0;qM$O4e*4530v(mMYra3=}~Vje3DI#t-AV7I3ZW)^7E?4X+dxYjod z*0#-*w?Uz*X~pigR`{s&Gqtom1ru2s5-ts+B-TKj&E?SJ!BKa$-HE7yO<(EM?~5mi3%ad`=#q#KT*p?u=*0H?5T97)&`JsHW|}BrW&r z?bpL1vxPeQ3sTOt&$yk;?$;Cf!)e)r)+XGP_plR~x4jd$t%;lB;U8WgkDi+XJk^kb z$(-U`E9C8^2qjz~#5=&^^nYGZ3voM`hLt|XY`4h|)0lNToo8;{?)Xs$DfK-a)ZiAj z=@YAvNk)PJ2}%PpebV##C=m#&ld|T>sfQ~HduDr#rl;lDVUL-O5|t2Gl&iclfBSI{ zM^O6QyYrS0)W4i0&Zy?Ik2zp(!A9>q?%}|iWe`_XHYEt#Z1v5d>`?Hx@855#^%%tl z>AkF=NrB^F)y9q4Vdg;n4MhE|)11KadDP`cy*Q*6gNpDzr8(qvMQ3!$OWsL1!FTE! zt&4hHwnmg^R^ERR9ikqH7qaYeml8hqdy;A`F;msN)f-fN zo#i!y6PZGu(GPMlG0Oe&89Y9BDu58-V&V6tB$mrwmwB=4MU)6#~F!`mHg# zMLi%SOvO(o!Ae5G#NHqHtwd-tAD3COX4Fr|o6~NZ4hvUyd}UMOKdG{-ZHcYB!tpYr z;U!4E`h~$*cwb=MEn?EDW)a1JB*ba{1mEUu>5BtzLt1&c-u0N%j}_c#=;tVNuh(q> zdYIJocquno9T-{^T8#O+d_T1Dq8C)?A{EtRC~v2#oRoEtr#t)tD(&vZQEmHpHZo+U zVBXwLMe!vRbZk-lw(R-#L)sxu0MxWI`H+zYye77&XDkd*f8CFVGkbEB^Jm{$@KnI0 z{>5*b*0s{EPKuZM24d@@0rPn+DiWoG%N|7_O!y{J-#TR5R;T4gjIDE{HMN+}g@#u9 zW^L6xPGvO1!E*`HGzAS?q*(c>w$i83JWnt8ze8j~#YcO`#b4*s0tPh~m#H5Z?6hA}{~|=6^)Rzogec zrNwU<);cXN+RX(Iy4Jkr0q(+>oTCzAI~4-nY7AVyIJyONi+?0G^W+jA{G}Js8JvBL z{Bf+C{K)xI-}VX+Th#zWbkmn#E0#=E73xNXZMj6rm%xmqwi;InZh+zbdgcMWCdU#Y zxvrZyM+vzE?*N-gaK_%v;1s#hgx7-UjWHWPE^+RHU>@Ck3dU1C5}PjGehKgaeJEgnv+>i#{C~)1K&1JK*0)2IA>r2ehZlqD?| z8IarIemcP0IuI^Ky#m`os@5gf34fw955P7`j6;*n;+_-^&FYv)vqz`O_++o-wQP^( zkD}y-(tOknY*=ZBqa%62`6F*3M?By|2+&W0YnShwZMS!KpEX+T5v4c5ZMc<|s6rG? zr#dRfK?z_nc3L&+rD$RBq}jM)dKOq0&r_XfiUiQ@|(6tvP}BI0~%<;!hsm0HO>LlKRH zzKg-R+YeO^LS{uniN>Ku!QSpe$bv!IL9f`v=7`MWj*K|V&|cW@uc<3n5yd05(WA9q zzM_p!hJqHzl?w-Z&du0tkjQPE)=b)8?XfFL70Iufj{gvG+AR`-kvbE#J5q^0QBwG0 zxvXxOA0+*U=1K?S@|I+0yrQE4(CV=>*{5$@)p^Zc4yxA*sxzl$XR$D@Gv;-vMc$I! zc!cMMKxWOcRrN~8h%|b(f>7nn67?!c-HOCyNb0orN;|zOzOd-0A8_p`q6iqaY(oPE zz)}K;_ic2JqHujXK4m9Bpw)@m&rx(cT zXcE@X+HW;&8B!CI5LA!2PGK=zw0s1kVU2o{I@-ikL}w!X?)Xca`UWjtY=C(jKvhgBk_Gz*JWAG#d4h^;~2!#dz^ z2kt48cwH>p;@Bi4b%Q|+EGp4l5Xb7FBrF0zpjjMmY}X1xYR*NTBcnthBpFK6daw{O zX^ocQA`umYHTO2{hBhxxs3VwN;m}>p(S?o->crusOW+c)y-9iF&AIimP=)JcNf~|& z!OVy^Tl>^1T=5aO8-Ua(JT~Bz-*`c8&cKaxsI=Og$4Y3>D3@DYSRTrU_1}5AKJbfw z65nux-W|{vw^5#Y!ZAr|d3IGER8yddj~($)7%T}!C40;wnnB!dyGBP8f!m7NP>+mb z|8Vn?0p(*vgrd@>hY>K5BALJTA$+o~Gx5oL;0%{fHz6-!3_h^&fD29cONJ$lja|Oq z9KnLF&%9w}gJU_w2cfdQ%rlix-o!5k*|7%)AS*I%3s8J<6&YScB390I;)=^M8yfz-cY-mneBjL6ti!l-Kfoj&~sogj^I5F1CJU) zKk!8r!ozI%?+-U-t`b#;Hb$5cbfJ^G(W!dy5$u2xBJ?=V!q910k;N6=dpO5c z&!&b3@E#4PM9QP4jm8+JyW=wLS3pE?fP?i5q)XhX1N&PS_x5-KecqZ(0_vjeY^~Z* zX-cx--)_^df>xxAFqv{?;TCzAVzX^3*Y<#co_Grk;4i zKvAds3jetsX6C?GgxZDP{=;^CeM3}=zPGJyq`E{7^?HsrN6m8HW~^Gd*4&arCkd;4 z<_^QKU*2I{2v)sey=tzY7Bad3zXO8<>dSoo)P1WPOPiGg&R3CRGL8`PxY6BW4|UvBa{V{@`aN+_Iia=4w|rRi5>vTqtAQuAi*pY7oMjsQMl2|X-xnDqs%jJV zan&J*Z`i2ivr!~b^zE{n0~_|~)-4V;^W_a6DQQt5js}d|ce%!nng#La_LDiEpi`1C z($Iv0foV>}ZI6cg>-gIMIfPPfaVIY6!kAsz_UR@GAFge}bs_bjBsqYOJk~kdWDbCW z7ny}%jcfFiblIT?y6`Xoy6xErvh9)b3)0X`qq$O96MY0I{M+7E@CFoI zD%atg27E-TifhfRS-BrBEe#cSBZ&kBAaTmjx}&5`)$G71SV)>}#I~ zOhE0iBBt>hXg5o>FVtbdkdyYT7f3cu3ux(lHX(4kPrVy+Nl0manvL{?B`5iE`=wXa zO-~P|(_&wdruZyhzz8z5PVt!!cooW1uJ6)-IQUUld=_{gbn;4I>`1%>y_8W-1kZvD zy7v6rn4vU01T69N0_aeHh*gEtix9kj=dFPIC;k^u{kI{|g?KoiaYE(ndw-L!+Ut_| zf-~IMFlry8g84Z=xSI?XMOpL!JB!cF02R8#Y_aHoA$r*ge|*HeA*SG5d;~vYz&U^2 z>vt{c5P^^o_t|1Ae8u_MBR;E8^$J&d zIj&6>VzZ$-C$=qnX;41eEL9$)^&Z-o^vQ|REbZY6QwAniPjvvRiqObTn7*NvmOnLr`rs%LPuv(qz8htsIh3*T6JsQXI(U(PjtdGj%#?;N+jm*S`4j~#J zndcK%1_lLM$?dBe;t3_j<^|**ZFxCI_H86PVziD*Mxe76=LuXY}U7 z(Fz!TKsF_2x44O5YkMMy(YqqdA=?0JQm#{Be7clK&dfHvLQ?lZ-+mzRNxW7EgGfVL z41WFck@mqFa;A~}q9ol=(QGZ0bdQ4`n)7Q;w|lh`TE5a&{Wp&OQZ|)w#cU>(I!m2^ zygC@jlV>QB9U+7z?J+td@mJ;Wozp1d6u78CQtY1Md21@wu`fQodW~u$3L!J5)Gzfe z)hFYfqaJ^|8v0v1U6D68!{Gn`0QVmOZDZ?Xs{h;3w5F~qF0vr@uimCS>6_)pJkFI9ECxkYVt!{EM(Vk7B|l`PoxIz%Xnz&i zSR)#@#+wT1+&lh(yJkd;AIYHs-NxzaerXQB1tN<7aX3a3TEmg!?)iE3^=9$XNu#BQ z4GtN1bUgY@r-D3q=H4)`D2XQkc(UL*c~$2hsjm+X||loU|E7hHM;nk zIzztz!~{}BH>x39%yi0qr6JbmV4-;>JpX9v4HOEfg>j+D)YlwLEf>ci$j3Bh+l>&} zdu|d6GLZdQC5w2oNi$#}4#jlvB3FE?K_J!Ck}~Kdn}Ds|Nld?Qr}8t&3=pB3RtNXI z&4Z*=Fv4^Im1DUMUevl^o7Q2&!1eeMN3pv?`$+mY&DV>y`;_fWc(@`jW6B^Y)|NIg z8)_*Nrs0rbwTix2(PzocBr3|1UK~CGP~O&lqO@G&YsrtA(0$ukPvxWDqfy1K9?Bz= z{m$r;t4m7@VYDG_<5yXY3$X__W%U#O*@I5x*vge-ayk?3(3vAR%C@=T>93@-(7st@EO>At$r-lGQ0M3(j-_w5fl-1{kfT>2DTg#i9O>Bu-M|0*GNOQS_ zT>evHRUze^sx_~)tDC$^xk|Ah(e5c9>~SI3s5c#6XN-Fy4wZQC_LEoQA#B&L3>l~q z{NsjnJX`Chv8i;nI5O-CVi`6btv-3_ojnAc^szf{12!1uT-_;dJ3_RaLp_PLWU?Ef zz6~S3B{%v=0uob9m6>r-ECTP2LZpx1qO``k1}?fw2Oe)@{ef|2tww^BG!4~8Djw!Q zL8sd3s1jacy*jTbp~*)JnhLTIuI2dg4=REMrY&+kWUf`;PU?T1pp<&Fvj=|Od8`hX zzO`{stQ;XhToGf-+bJ=%kg*%1YcN1}8To09zE`~fW}ve7>>BYy^o)x-&rC?}(s}dq zgIb-?+%v0ffB=fxCv0g)Ge+ODS)>}1B;&ym5%BXhL3`u z$R1pNG61~kDk3PKZ{%KJ9zUk$O|lAC>SCWi)#+cu6+f5oEwn%iPYfyfOVSzLJ?Qt+ z%Ez%^e{qYmB2RbX6HGx#BzGHyy+3cT8SoHM5RD7L#BGARzCMQT#8SQH$aqW$iV>)n zv_Sl_;b{(P^=#?ziESX3g4V8o0)TTMD*^(&ng!|ZJC5s%WD=-G(Pt(%JeCE~tSMel zdqQx~#3%y%&rwO=g5@GYGyQKZUTEUULktaX(J4=HNtqC=`l95;P*h1NIf|cnsXo!P za4LVfa>6UhaK`mg&WE|Ye&S?1NW zbtspBz)$+1L`BMpkM$B%cJb>!+J6+*7qqip9&zt|e>5oJZJUo#<{83JX(f7nRj8l0 z@)I~|ypeJUk2#{Do(+Ses0@AHacEbvCc}>Br&2W^yID-w|G_=n zq_s-T+R~lJkshu(a^grZE4(aqN7Z}P(0q=DCJFn=!T^yqjtJ}PI!AuM*ZS%$tl_Ef zILq`?78Puu;X22=fvS7~K_;`ybn&oV%eJXmsLNWbP4h)eBi;Ee?|d_j8YI$AeYAsO z5UEsi>dm^-k8I{HV#tjo36|EfEd8;cetgtgy_1Y&Ov~o5e0H(91s-^e9!tFZUQ6Dg ze_(YR&}2T{Lo(_#F~EG{zHUr3-0)aw(`1FuvWvx-W%I!rpGE(3J~Js{BT1@F^rs)p z!i@-frXYgAYIWW>4W=N+ZdE8bcY4Lx)c}lHm$n^YTwtVp89=rDVx3`JAf!~cka9xY zzb~PDp74Ss(rV!HyZKc(ydX(_l|3#$aN7(XU=}I74_F28Ua)fGjjdBlWKwt_|2hri z0z1et$l!5Ndn@T&e#pPd--x$c-hT80N`earX-)>v?_WaL<6_@OFPOkaEtps#lEUkK zD}NY&)$w1q6>x#iWiq; zul&Z#6^|wFY#qY{=XeX#{HW%{09_|%5L09mYZ_b3Ac9+jlk*qjv}T!&hTG~0(LjmJ zgy(wn-DHC?!ac22lPW_6VK3`U%@VsQ%!hx(;T^5ljhLc$!I}^E$E^!zL|?XD3-`Mx znV(RC_%b-ufaja@B&+h2rtIFZhU|Q?l^+-1Bh60soSX{kggO++M6u6)%JJ9sk6qxD zS_Uqej#wk4@3<3%kzSGEjY*%TqSndvf4lrLI4^z{g`xG7>rZS^h&Lo@%oL;j`Q z*rr|2qn%?K;Xf~rBez*1 z7@_Hb8HmH9U{>|Vo{7>wR{p9CJZ=nOysh|kKI>jyw9eUmChBXpW-aj5RdF1K-}T!8 z<)%1pNoKPs_eDd!C*E^~_+@%%67@0$j{{)YZ_)BnM+SwxusANpz1 zH+Ylm7wS?HXsF(zhQ>jJFvC1r_s&HXlfJ zuRF^P5C#pNZ5_UBj{0wZeAv`@=#+gqkG2_&Z>3**)V`VP{*sI{stIS~+I6gBnHY3Y zcye6-gCS&OccEleTjP}qrmB5ZL2&0paqdX6r1EI=Xwr1%DX3O1uf^EvS(-i@dYVR! zl)C$1TMPDUar~2cnhb6E-Lm5B`4=r4f<^H1`Z8Dh!-#Er))OXE6NgM%5z%DvJ|nxX zgI?Wm(6m%LlT71rHi}m6XPanaO82s*LQ|uw5SRm)a!1V7N^6rwYYU7^e(5qStX2~x zPD)LSy_)e6rafS2hflOin7+;J!*JMgjD}3QD{L>2&CRalbq10SXMXC}qe!)nVU=l> zlrB*u4|BE#wW&E;E{6u^aCs$Z%zZb-a5N!~aTd>cPZ3#JieNACNvH`PwsniH;!-em zsuz95EvAgvyZY*n+3}T9L9`ol&m*r_v^XdB9`Sc~Lc)pI&N^+RF3i>V3nIe9;^4_s@J&Ts_motB-I<&^1FG!IA$ z`=TnIhT~eEu99XzMrIx*Q&T3Iw!U8cdx?l918l2WGI4KVH51cp*U?1dH|+rJ3RwtO z1RP)mZq;WXva9ddCLbu1bd8{AEy~BO@F_G#H{LBRCFZj6UB5NF_gich;(r?c+gp5= zoVJds{;##A&H?^V4>C74l!8q- z5pezJa64LL%K*^n3Ppx*t>PzD9tF%t2on%goZG9l-?9$q@k~Lz^!js-u`4kNtX5vd zPdAFXE`zw=^3$>hNkbrXr@zmeK@T z0OZU90qLsfa^d7*pmHX;T|VZrf_qQaq*;7?g>;8mr?wfPE_I>?D*|ufUA0HNA?M(W2%t z9BFcf$Xuue^6)dloR;MaMa}R9SfDo*`pj-g)N$-)3^lScCE=Mg&2C4rC8=Z_4y`m; z$g`xMtv4(dtoXK|buGtoY}UGh;FRLpHoEauGl1qNUMk>@95|LJc0GCn4D<;i%B%2J zQ`T>S@zGgmdZ(yEi1b*75O5k_!||W7@HD5(AQdf!u3lS7+`wiz-|ecwzJ9MWd+Q|B zZ*S+XsHeAGNhxa!OB+gE3+vx~BrK{^qMHiZ|5T`jH`XQoTQam^aYz5TQN#~-jIvax9h3?f3nDsnRW*P(2sJLiN!nATsN zq>U0HK`|T0#>Z*2Wu%oID`EjdAl0_S7jnBs20(x0#IRI)MKwnf)xfQisaju2Hs|?= zNVb8pNJK%Ye6SvjoWcziw;%w^MQ)umHTwaXlOe?gfq|uuFW{jBtlHG+&A|Lw)Gy47 zy9UbTVYm>xj-Sf0wWyF1(3w;;a=44;PJAt#Dx`2T2E*%gleS`OQTTq7QL;b z%qu=Q9L&>{W2{7eexF1{Jytrl(+kYso>NU-vSj4VHp_3ef&Zs%v@I=vy}qfk;ueck z(5-MkJ*aFgwsO+Jkx_VqGXu1t;Op6|X^^0z3AuHAzO}pCE;&jx%epzc=A@?yi7dC( zmHpHnVz~25kPwm1V9cy@Z-$4Pxvl;rt@lCG2MQZi=ezRKLAmBMsb63a`HnZCt45$d z@mTIiUgyUP->Ed(#vrOA&P94~)3QQ#vvX?G__e0E zUU(;IZ4KnHn7s$8p<@`hC&e14?DRsjI8{(#F{ptH4Z& zI~ll5SJYr4RX$lZY>^$pG=z%0;7_yjFpi=89GeT@VWmTsi9E)^-V;L}kEk@6<~hPu zJ|lRj;$YH3r6p8Xwk~1s0b`>0r0en{@u<@%6O)aK!_I*T?{(p9d5`m3zHT)boQ~e9 zPxU7Hsa$+!*xK!BzjaP?-Rl@!C8Wc$cMJVh3})ENpiFhRmLpeu_2D^UD2?$b`7GPA zHWyIl$vw-U-NRG)s*YR9eZt=CL8V8jvok%(J?qg_DW#FEzS+Nax>Z$e78}srPQ_k<1l{tS@(Cnku*zd<()xtI9h*Pa z0e#5mO(H=4P_sQh|GM&B@%&2=s{Y(D-53F0kj>dTz$yst#)1~!x;?XgIz zFkMfmO|y>i&K&8apH{PdH;1L zPu9`=`s5k0XbW0nI39MA=e8v$v@LYO;o=##B2^M z23?)AWC=+@2;;d$3YibcBUyi+58v6@Wy$F?f_<`w8LpsH=bg6x+F}clh(A}WhNbsm zoyR?_*SHQv$Dx#%E!IyH1Unqfpsb;bTq@zvIf1mPXhDPvZiPku+6Oj^HBmSLXlO|RYU6QY17SxviJ)2*Mn8v#hi5{pdb9d_pvCX! zmh(-Qu`q5env+>pLP3u-M&s&yf^wCswh>hszRpCT=IIUyO%<9XP0z+Z0BQkv!i!g< z#vNtk;wArN*zQd!suDo`sN9q%S}89Pk9wp;kaI>Q-TELavG!=Ixlt&I#3&4*P#p}+ z4-{dtJLOhoJ4`6h-V0A>$SAn(XgS}gAdAl0e zWE`qp3sbNHbNYsfr4t7MH5%MprxSYu9|z>6>ct4z9O%apAp4IBqJ@UG)vUlM4`-c6 zIM_@xU~?c+reP9XR=%!_R z__VP|kDm%H((4rq`>H^y9y2$y_5&wI;M|6!OfzS!k+X{Urxa>0hhD7(^#%KY;a0Q3 z7sNBb#zHJn6hM|(2;+dqNT4_OA`NiEy6gDc60yO0E%CNGz{Ipz&A8(EeWCyQu zX_8pPl0f^ZQH)2wke+efZty_rZ4YUmEmU;kto8S)*EICxpo)K}dFAUCtc*>I#NmZn zt|E1oL!-!&uBBro&V`dLlA!5uJLssdwoU?B<3=}W0GbH`EtmprPu-g=4R_N>{c`qm zjgs?>bD%r?<0aVq4iM0kRbTiFh`Q9INo(qw8*ABoWhOH?a|QOn7RFje>K*5-po{6E zNaNLsF;@mVl=7LgoVSJ>GC5B4RBfGYJ)qp2{-{)P=2hghv}D7;ZY+MYs?tomO5;~+ zozN5|e4?EJc^CI$bX%}Tf^r&$F$aifajLvLe8{IjG)$#naG}Hn zE*rn<>apC&(4zh%AOW70GefWj52Pgj><5i6Q9Ids4(r1xzaG+B-9e?u`C*^4zC#7cm zI~+d`T>s#jjQ8nTD8&W-%LGE|rRw1kzlzZdcA4X{FyalJ>gX8%jp>B&qpH*)pW%1j zfZhxs^j8Hv8+hX{gtRS5xwvN+XXi`jrUF(z)JlLPHMqnA$xDTcD+z^DzVoVs91P-2 z9cJD`i6)bB-yGGdj+_&%l=^lu^vP#Pn&NG0M?L1li(lf_BM4nm?%B5dGGh>hI&e!9 zaliy6XkEYaHr>T0lo{zB4HO+WNjV#SP1hi}}ZZq_8-w z@|<52AVgZ8qv0*^e%W;_AK&(Y>K!c#7-^e(0exV%2vWbnU7_n0Y7BXL&4-4J*lb9g zQ&-)2s76nAsrx~2X5_EUOc%1?og!t3iZx{7%r)ZnYAr|+93-kV#N1=k$bOWhBY3+Z zX-IYYSLtX1^F*x7-aX-`enAx^6pTE8j|~?vN?fSC(&GFxUYC$8r_xwbz(+v5Fb zAcQiompWLl#7(Le- znnQBmKUl2^rOYkCinp`WEwv&2^5gI^-guC1e{*Dt^#N*HB%Tc&-MS;%L^eI8d~JCx z{JvVjYo(x6HK4Zk*xu~h6z{@=vsoq?>v84JaIsk)vM?*+<4Ek&)1FEm^whB~RY6Gn zj_XaImops3!%t)AH&IEOK+~`rc2~AQienRp@ViTV9s2u}`Z&s9xl=pj`pwu4D-n;1 z6o!_`@vlA^emU?d-`AbLRs-2EsE+=?Nj%#GWiaZau{ETPDkPDYOqwN4Yp|!itMQDr zLe6^PQ^)vWyU7$6WQbJp09-;1Xd=;}(#~6PlCZz7eYq$1J@4}Zas>y7sTg3gY{x8` z1F}PfmGBfbQ;st8p)_atEmQ^ATwRG@d8j_GMlmyNyA82c3n7xUe!Ditn_-N zkA=7iOy5FOpB7HeP4NEwa^VKETJ@-YBF`NjwgvZ`B!@TZtJ*FI{0lat#orddS`u}4g?^pO0j=Wj1+FoK~qBVEmdOWuop35{kY2fo!qT=CJu#@?dRc&O-l#LEO ztwv%lT&EMzCiI3UYD7&=eGRUNPF^vX&%s`*{tg zVBfz!8gPjekq3+18_Winpm86+yqi(maxy!&yjh0$ZRUpkmxcZn_xnEz{&&!C*mz~| z+w>ZA?Rjmb)6gPiNooioUzEg_RRp#PLr_>ucUZ~TXtix8kIZ(DIOz3Jy1CVlU!A%- z-!P-lSPBgbhC>3>{qfltD7d$;lLGli2NrtW@~)suPp=5X`4Lia79RsuNRCD6{3sf^ zVhmN>un@BniEf&0)rl$5w5DC_?0q}na&#fAkC!VNhSW3rIx9n1wnGEH0|eSOzn?wo zN67q&Cq2@zqIxGD>NSw=)uO^+h$fTkDUaoMJyR`?==1;e+`kR{@8|yY9=-)TKYE|S z@0iQ72a=1ol6x~oQaHlw*T5|5@q9yebMeVXVRKADe>pWsTK+n65S+-)4^&(P>i0wX zbklIHevz_1L&@~$SPTJXDN=imqAdw&OhF`&C|wDKu*yrAz|CaTyi2Pw%ePVubf{AV z{jwn1&cmLswd^o#>=njPQ_OA;W{bD|!^|?0yUrAqV*uwC1z>hd;lxQ7&pPIgZr)@8 zOR^v`kOmc-;$cxj#-c#`(;NrD-~GCwnAL~*&DNrCiFN<`jimkcMwUi#N_JDh_qK~{ zFI%Pjyw4{rZ6r5oPNzaoOMKu8!8@9P2o?rBp1(L9~l2`>4N+ym;b|q z{BG{QJ;;~RaC#kNRsl;+@5P8V<6kbGxQptQLs|WV-E#coEGXM?GY0ev)C|ur7Y}1w zZ~4qT3=uBDrTBdsE)&LLfJ6{OO{pC&7>I;Eo$-!9vP2 zD&_KyC~hsi2-7b(e?R9&V$D40TaSu*E5CZwpEbGP%K*QZekNEge5l|7)>>s=eYR77 zCNO@;>hYW|g76db!28j$~g<9lKpl4S5b5*V0}PlZlyF)do-4+olS# zfmoF&jpxxaCGt0q^NzLD2R$j?&EJq0?X5{o01SuE^F=k050BWV40ZUnJJcmFQ0Qc! zUNkNgzByg3a-%rr%MZMXcCDyOcstDr=F#F>`8~?#sQ!55(MW!*I9Ssa1iyYts%xAj z%MNRO0AE~y9p_Py5~`}Fo1||`$S@fzi4uJJCV>l==~Xfx_zF&E|6XA^ZPwK)?IP ze|118AC;4wm4PijtDj*^8hxr8^c^97xbbH%UF+N&IrHG0Ix^Q*P#ifT!BCQ?Ck_pF z#t{VKTy}2+>DAG-FbFnW3x{sDKmU79*g^S~gOltX;>uFpn>+kpDYWn5@6%)cz;WMllz+qjKglxhncioI{9!VA3wis^^gdVQ zJ;nP_+&>iDZ*Spmihl;?zK6dLuloZpLH!T>e}e7agWqqG{Q=_>{2Tmk%j`YF``w;D z3@L>FX83FG=RN-YM!+9@C(%D$(7(3@-oxLoRsVsjll%kz@0IKK==bZLf6$6#?=|jk z8ULQ(z5M?}5JdN$;Ll{>J{fEVh{yocYDf6D-y~h1RK*sQ%;CD6tE6aNY`G*DR zZ9Vvp;{R`Y@_miotFk{_P)z@w>yJiz&-GsE{NVy({wLQjh5Oslz9)D;fd3&dV|h>T zyLtXA%lqN+4~snOKf2n#+UeI|`Mw(O2Z}!gp=|%G#-H~6Gi*qSg1qIR{mMW?0dN5U L0Pto1b@u-N(easZ literal 0 HcmV?d00001 diff --git a/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js new file mode 100644 index 0000000000..f49f9f85ad --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js @@ -0,0 +1,92 @@ +import { describe, it, expect, beforeAll } from 'vitest'; +import { getTestDataByFileName } from '../helpers/helpers.js'; +import { getExportedResult } from '../export/export-helpers/index.js'; + +const THEME_KEYS = ['w:asciiTheme', 'w:hAnsiTheme', 'w:eastAsiaTheme', 'w:cstheme']; + +const collectInlineRFonts = (doc) => { + const found = []; + + const walk = (node) => { + (node?.elements || []).forEach((child) => { + if (child.name === 'w:r') { + const rPr = (child.elements || []).find((e) => e.name === 'w:rPr'); + const rFonts = (rPr?.elements || []).find((e) => e.name === 'w:rFonts'); + if (rFonts) found.push(rFonts.attributes || {}); + } else if (child.elements) { + walk(child); + } + }); + }; + + walk(doc); + return found; +}; + +const countAttr = (rFontsList, key) => rFontsList.reduce((n, attrs) => (attrs[key] ? n + 1 : n), 0); + +describe('SD-2894 — theme rFonts preserved on DOCX round-trip', () => { + const fileName = 'sd-2894-theme-rfonts.docx'; + let sourceRFonts = []; + let exportedRFonts = []; + + beforeAll(async () => { + const sourceXmlMap = await getTestDataByFileName(fileName); + sourceRFonts = collectInlineRFonts(sourceXmlMap['word/document.xml']); + + const exported = await getExportedResult(fileName); + exportedRFonts = collectInlineRFonts(exported); + }); + + it('preserves the inline rFonts count on document.xml', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + }); + + // The ticket measured the bug by counts dropping (66 → 31). Asserting "≥ source" catches the + // regression direction without over-fitting to leaks from the cascade that don't affect Word + // rendering (e.g. a docDefault eastAsiaTheme appearing on an inline rFonts that didn't have one). + it('does not drop theme attributes that were on the source', () => { + for (const key of THEME_KEYS) { + const sourceCount = countAttr(sourceRFonts, key); + const exportedCount = countAttr(exportedRFonts, key); + expect(exportedCount, `expected ${key} count to be >= source (${sourceCount})`).toBeGreaterThanOrEqual( + sourceCount, + ); + } + }); + + it('preserves the theme value verbatim on each run that had one', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + for (let i = 0; i < sourceRFonts.length; i++) { + for (const key of THEME_KEYS) { + if (sourceRFonts[i][key] !== undefined) { + expect(exportedRFonts[i][key], `run #${i} ${key} should equal source`).toBe(sourceRFonts[i][key]); + } + } + } + }); + + // This is the SD-2894 customer symptom: theme refs in the source were being replaced with concrete + // font names on export, defeating Word's per-script font resolution and causing Times New Roman + // body text to render as Calibri. + it('does not substitute theme references with concrete font names on any run that had a theme reference in the source', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + const themeToConcrete = { + 'w:asciiTheme': 'w:ascii', + 'w:hAnsiTheme': 'w:hAnsi', + 'w:cstheme': 'w:cs', + 'w:eastAsiaTheme': 'w:eastAsia', + }; + + for (let i = 0; i < sourceRFonts.length; i++) { + for (const [themeKey, concreteKey] of Object.entries(themeToConcrete)) { + if (sourceRFonts[i][themeKey] !== undefined) { + expect( + exportedRFonts[i][concreteKey], + `run #${i}: source had ${themeKey}, export must not have a concrete ${concreteKey}`, + ).toBeUndefined(); + } + } + } + }); +}); From f046df9c04aa69be98ee0f9cfee1258548fea922 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 13 May 2026 14:02:14 -0300 Subject: [PATCH 2/5] fix(super-editor): respect explicit font changes on themed runs (SD-2894 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Luccas's PR review feedback on #3225. P1: when text imported with w:*Theme rFonts is later given a concrete font by the user, mergeFontFamilyPreservingThemeRefs unconditionally restored the old theme reference and dropped the user's choice. setFontFamily('Arial') on themed text appeared to do nothing on export. Fix: gate the merge on whether the mark-derived value still encodes to the same OOXML mark as the run's existing fontFamily. If they match, the marks are a faithful re-derivation of the import and we preserve the theme. If they differ, the user has overridden — drop the theme and keep the new concrete value. DRY: FONT_FAMILY_THEME_PAIRS in the plugin duplicated FONT_SLOT_THEME_PAIRS in style-engine cascade.ts. Export the cascade constant through @superdoc/style-engine/ooxml and consume it from the plugin so the four-slot mapping has a single source of truth. Tests: two new AAA cases in calculateInlineRunPropertiesPlugin.test.js lock both directions of the contract — preserve theme when marks re-encode the import, drop theme on user override. --- .../layout-engine/style-engine/src/cascade.ts | 4 +- .../style-engine/src/ooxml/index.ts | 9 +- .../run/calculateInlineRunPropertiesPlugin.js | 71 ++++++++++---- ...calculateInlineRunPropertiesPlugin.test.js | 95 +++++++++++++++++++ 4 files changed, 159 insertions(+), 20 deletions(-) diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index daa0e68543..dce9cb82d0 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -114,7 +114,9 @@ function isObject(item: unknown): item is PropertyObject { // // Pair `` with `Theme`, except for the cs slot whose theme attribute is the // lowercase `cstheme` (OOXML inconsistency, not a typo). -const FONT_SLOT_THEME_PAIRS: Array<[keyof RunFontFamilyProperties, keyof RunFontFamilyProperties]> = [ +// Exported so super-editor's calculateInlineRunPropertiesPlugin can consume the same +// list instead of duplicating it (SD-2894 follow-up). +export const FONT_SLOT_THEME_PAIRS: Array<[keyof RunFontFamilyProperties, keyof RunFontFamilyProperties]> = [ ['ascii', 'asciiTheme'], ['hAnsi', 'hAnsiTheme'], ['eastAsia', 'eastAsiaTheme'], diff --git a/packages/layout-engine/style-engine/src/ooxml/index.ts b/packages/layout-engine/style-engine/src/ooxml/index.ts index b486daed07..e894b5a955 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.ts @@ -5,7 +5,12 @@ * This module is format-aware (docx), but translator-agnostic. */ -import { combineIndentProperties, combineProperties, combineRunProperties } from '../cascade.js'; +import { + combineIndentProperties, + combineProperties, + combineRunProperties, + FONT_SLOT_THEME_PAIRS, +} from '../cascade.js'; import type { PropertyObject } from '../cascade.js'; import type { ParagraphConditionalFormatting, ParagraphProperties, ParagraphTabStop, RunProperties } from './types.ts'; import type { NumberingProperties } from './numbering-types.ts'; @@ -18,7 +23,7 @@ import type { TableCellProperties, } from './styles-types.ts'; -export { combineIndentProperties, combineProperties, combineRunProperties }; +export { combineIndentProperties, combineProperties, combineRunProperties, FONT_SLOT_THEME_PAIRS }; export type { PropertyObject }; export type * from './types.ts'; export type * from './numbering-types.ts'; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js index 4d5eb1952a..7b43309b60 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -2,6 +2,7 @@ import { Plugin, TextSelection } from 'prosemirror-state'; import { Fragment } from 'prosemirror-model'; import { TableMap } from 'prosemirror-tables'; import { decodeRPrFromMarks, encodeMarksFromRPr, resolveRunProperties } from '@converter/styles.js'; +import { FONT_SLOT_THEME_PAIRS } from '@superdoc/style-engine/ooxml'; import { calculateResolvedParagraphProperties, getResolvedParagraphProperties, @@ -28,24 +29,21 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([ export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']); -// SD-2894: when the plugin overrides a run's fontFamily from mark-derived values, the -// mark only carries the resolved CSS font name (`ascii: "Calibri Light"`) and forgets the -// theme reference (`asciiTheme: "majorBidi"`) that came from the imported ``. -// Without this merge we'd export concrete font names that defeat Word's per-script theme -// resolution. The pairs match the four `` slots — note `cstheme` is lowercase -// (OOXML inconsistency). -const FONT_FAMILY_THEME_PAIRS = [ - ['ascii', 'asciiTheme'], - ['hAnsi', 'hAnsiTheme'], - ['eastAsia', 'eastAsiaTheme'], - ['cs', 'cstheme'], -]; - /** * Merge mark-derived concrete fontFamily slots with theme references preserved on the * existing run. For each slot where the existing fontFamily has a theme reference, keep * the theme and drop the concrete value from marks; otherwise take the mark value. * + * Only safe to call when the caller has already verified that marks are a re-derivation + * of `existing` rather than a user override — otherwise this silently reverts user font + * changes. See `marksMatchExistingFontFamily` for the gate. + * + * SD-2894: when the plugin overrides a run's fontFamily from mark-derived values, the + * mark only carries the resolved CSS font name (`ascii: "Calibri Light"`) and forgets the + * theme reference (`asciiTheme: "majorBidi"`) that came from the imported ``. + * Without this merge we'd export concrete font names that defeat Word's per-script theme + * resolution. + * * @param {Record|null|undefined} fromMarks * @param {Record|null|undefined} existing * @returns {Record} @@ -53,7 +51,7 @@ const FONT_FAMILY_THEME_PAIRS = [ function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) { const merged = { ...(fromMarks || {}) }; if (!existing || typeof existing !== 'object') return merged; - for (const [concreteKey, themeKey] of FONT_FAMILY_THEME_PAIRS) { + for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) { if (existing[themeKey] != null) { merged[themeKey] = existing[themeKey]; delete merged[concreteKey]; @@ -62,6 +60,32 @@ function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) { return merged; } +/** + * True when the mark-derived fontFamily value encodes to the same OOXML mark as the + * run's existing (imported) fontFamily — i.e., marks are a faithful re-derivation, not a + * user override. + * + * The encoder resolves theme references to their CSS font name. So: + * - import case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Calibri Light' } + * → both encode to { fontFamily: 'Calibri Light' } → match → preserve theme. + * - user-edit case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Arial' } + * → existing encodes to 'Calibri Light', marks encode to 'Arial' → mismatch → user + * has overridden, drop the theme and respect the new value. + * + * @param {{ attrs?: Record } | null | undefined} markFromMarks + * @param {Record|null|undefined} existingFontFamily + * @param {(props: Record, docx: Record) => Array<{ attrs?: Record }>} encode + * @param {Record} docx + * @returns {boolean} + */ +function marksMatchExistingFontFamily(markFromMarks, existingFontFamily, encode, docx) { + if (!existingFontFamily || typeof existingFontFamily !== 'object') return false; + if (!markFromMarks?.attrs) return false; + const markFromExisting = encode({ fontFamily: existingFontFamily }, docx)?.[0]; + if (!markFromExisting?.attrs) return false; + return JSON.stringify(markFromMarks.attrs) === JSON.stringify(markFromExisting.attrs); +} + const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys'; const COMPANION_INLINE_KEYS = { fontSizeCs: 'fontSize', @@ -570,10 +594,23 @@ function getInlineRunProperties( const valueFromStyles = runPropertiesFromStyles[key]; if (JSON.stringify(valueFromMarks) !== JSON.stringify(valueFromStyles)) { if (key === 'fontFamily') { - const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, editor.converter?.convertedXml ?? {})[0]; - const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, editor.converter?.convertedXml ?? {})[0]; + const docx = editor.converter?.convertedXml ?? {}; + const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, docx)[0]; + const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, docx)[0]; if (JSON.stringify(markFromMarks?.attrs) !== JSON.stringify(markFromStyles?.attrs)) { - inlineRunProperties[key] = mergeFontFamilyPreservingThemeRefs(valueFromMarks, existingRunProperties?.[key]); + // SD-2894 follow-up (PR #3225 review): only preserve theme refs from `existing` + // when the marks are a faithful re-derivation of the imported value. If marks + // diverge from existing (user picked a new font), respect the user's choice + // and drop the stale theme reference. + const existingFontFamily = existingRunProperties?.[key]; + inlineRunProperties[key] = marksMatchExistingFontFamily( + markFromMarks, + existingFontFamily, + encodeMarksFromRPr, + docx, + ) + ? mergeFontFamilyPreservingThemeRefs(valueFromMarks, existingFontFamily) + : valueFromMarks; } } else { inlineRunProperties[key] = valueFromMarks; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js index 5b510c94e4..dae94be0ee 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -857,4 +857,99 @@ describe('calculateInlineRunPropertiesPlugin', () => { } }); }); + + // SD-2894 follow-up: the theme-rFonts preservation must NOT silently swallow user font + // changes. The two branches below pin both directions of the contract: + // (A) marks just re-encode the imported theme → preserve the theme reference; + // (B) marks diverge from the imported theme → user has overridden, drop the theme. + // This is exactly the case Luccas reproduced in the PR review. + describe('SD-2894: theme rFonts vs explicit user font change', () => { + const fontFamilyEncoder = (runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + // Theme reference encodes to the CSS resolution of that theme. The customer fixture + // had majorBidi → Calibri Light; we use the same here to mirror the bug. + if (ff.asciiTheme === 'majorBidi') { + return [{ type: 'textStyle', attrs: { fontFamily: 'Calibri Light' } }]; + } + if (typeof ff.ascii === 'string') { + return [{ type: 'textStyle', attrs: { fontFamily: ff.ascii } }]; + } + return []; + }; + + beforeEach(() => { + // Style cascade has no fontFamily for this run — the only sources are the run's + // existing runProperties and the mark-derived value. + resolveRunPropertiesMock.mockImplementation(() => ({})); + encodeMarksFromRPrMock.mockImplementation(fontFamilyEncoder); + }); + + it('preserves theme reference when marks re-encode the imported theme value', () => { + // Marks decode to the concrete CSS name that the theme resolves to — the import path. + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', cs: 'Calibri Light' }, + })); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + // Trigger plugin re-evaluation with any benign mark change. + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual({ + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + }); + }); + + it('drops theme references when the user explicitly sets a different font', () => { + // Marks decode to a CSS name the theme would NOT resolve to — the user-override path. + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial' }, + })); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + const ff = runNode?.attrs.runProperties?.fontFamily; + expect(ff?.ascii).toBe('Arial'); + expect(ff?.asciiTheme).toBeUndefined(); + expect(ff?.hAnsiTheme).toBeUndefined(); + expect(ff?.cstheme).toBeUndefined(); + }); + }); }); From bec7e63ed88b59b8af2947a357c9228f45f65bfe Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 13 May 2026 14:16:53 -0300 Subject: [PATCH 3/5] fix(super-editor): narrow user-override gate to primary fontFamily slot (SD-2894) The previous gate (f046df9c0) compared the full encoded textStyle attrs between markFromMarks and markFromExisting. That over-fired on imported runs whose existing.fontFamily mixed theme refs with a concrete per-script slot (e.g. { asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }): the mark re-decode produces a full concrete shape including a `cs:` slot, which the encoder turns into `csFontFamily` on markFromMarks but not on markFromExisting, so JSON.stringify(attrs) mismatched and the theme was dropped. Browser round-trip of the SD-2894 customer fixture: 65/65/65 -> 62/62/62 theme attrs, with 3 `w:ascii="Calibri Light"` substitutions. Regression of the original PR. Fix: compare only `markFromMarks.attrs.fontFamily` against `markFromExisting.attrs.fontFamily`. That is the primary user-override signal; per-script extras don't indicate user intent (the toolbar sets all four slots to the same family). Confirmed in browser: - customer fixture round-trip: 65/65/65, zero CL substitutions - Luccas user-override repro: setFontFamily('Arial') -> exported Adds a third regression test pinning the mixed theme+concrete-per-script case that the suite previously missed (because the integration test uses isHeadless: true which bypasses the plugin entirely). --- .../run/calculateInlineRunPropertiesPlugin.js | 18 ++++-- ...calculateInlineRunPropertiesPlugin.test.js | 64 +++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js index 7b43309b60..25747fe71b 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -61,17 +61,25 @@ function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) { } /** - * True when the mark-derived fontFamily value encodes to the same OOXML mark as the - * run's existing (imported) fontFamily — i.e., marks are a faithful re-derivation, not a - * user override. + * True when the mark-derived fontFamily value encodes to the same primary ASCII font as + * the run's existing (imported) fontFamily — i.e., marks are a faithful re-derivation, + * not a user override. * * The encoder resolves theme references to their CSS font name. So: * - import case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Calibri Light' } - * → both encode to { fontFamily: 'Calibri Light' } → match → preserve theme. + * → both encode to fontFamily: 'Calibri Light' → match → preserve theme. * - user-edit case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Arial' } * → existing encodes to 'Calibri Light', marks encode to 'Arial' → mismatch → user * has overridden, drop the theme and respect the new value. * + * Compare ONLY the primary `fontFamily` attr, not the full mark attrs object. Per-script + * extras (`csFontFamily`, `eastAsiaFontFamily`) on the mark are derived only when the + * input has a *concrete* per-script slot. Existing run-props can carry mixed shapes + * (e.g. `{ asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }`) that encode without + * `csFontFamily` while the mark re-decode adds a concrete `cs` and therefore does + * include it. A full-attrs JSON compare would falsely flag this as user override and + * drop the theme — the regression caught on the SD-2894 customer fixture round-trip. + * * @param {{ attrs?: Record } | null | undefined} markFromMarks * @param {Record|null|undefined} existingFontFamily * @param {(props: Record, docx: Record) => Array<{ attrs?: Record }>} encode @@ -83,7 +91,7 @@ function marksMatchExistingFontFamily(markFromMarks, existingFontFamily, encode, if (!markFromMarks?.attrs) return false; const markFromExisting = encode({ fontFamily: existingFontFamily }, docx)?.[0]; if (!markFromExisting?.attrs) return false; - return JSON.stringify(markFromMarks.attrs) === JSON.stringify(markFromExisting.attrs); + return markFromMarks.attrs.fontFamily === markFromExisting.attrs.fontFamily; } const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys'; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js index dae94be0ee..6789b1f265 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -919,6 +919,70 @@ describe('calculateInlineRunPropertiesPlugin', () => { }); }); + // Customer fixture had 3 runs with existing = { asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }. + // The marks re-decode to a full concrete shape including a `cs:` slot, so the encoder adds + // `csFontFamily` to markFromMarks.attrs but not to markFromExisting.attrs. A full-attrs JSON + // compare flagged these as user overrides and dropped the theme — manifesting as 65 → 62 + // theme attrs on round-trip. The gate must compare only the primary `fontFamily` slot. + it('preserves theme refs when existing mixes themes with a concrete per-script slot', () => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Arial', + cs: 'Calibri Light', + }, + })); + // Encoder mirrors the real encoder: theme resolves to primary 'Calibri Light'; + // a concrete `cs` slot adds `csFontFamily` to mark attrs. + encodeMarksFromRPrMock.mockImplementation((runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + const attrs = {}; + if (ff.asciiTheme === 'majorBidi' || ff.ascii === 'Calibri Light') { + attrs.fontFamily = 'Calibri Light'; + } else if (typeof ff.ascii === 'string') { + attrs.fontFamily = ff.ascii; + } + if (ff.eastAsia) attrs.eastAsiaFontFamily = ff.eastAsia; + if (ff.cs && ff.cs !== ff.asciiTheme) attrs.csFontFamily = ff.cs; + return [{ type: 'textStyle', attrs }]; + }); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + const ff = runNode?.attrs.runProperties?.fontFamily; + expect(ff?.asciiTheme).toBe('majorBidi'); + expect(ff?.hAnsiTheme).toBe('majorBidi'); + expect(ff?.cstheme).toBe('majorBidi'); + expect(ff?.ascii).toBeUndefined(); + expect(ff?.hAnsi).toBeUndefined(); + expect(ff?.cs).toBeUndefined(); + }); + it('drops theme references when the user explicitly sets a different font', () => { // Marks decode to a CSS name the theme would NOT resolve to — the user-override path. decodeRPrFromMarksMock.mockImplementation((marks) => ({ From e06c0b8317ba0576b32b144fedad12c736659b34 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 13 May 2026 14:35:36 -0300 Subject: [PATCH 4/5] test(super-editor): mutation matrix for SD-2894 theme rFonts round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 13 parametrized scenarios pin every shape of existing.fontFamily x mark-decoded shape that the plugin must handle on round-trip: Group A — theme preservation when marks re-encode the imported value: A1: all 4 theme slots A2: 3 theme slots (ascii/hAnsi/cs) — SD-2894 customer fixture shape A3: 3 themes + concrete eastAsia — the regression case the gate now handles A4: ascii-only theme A5: hAnsi-only theme A6: cs-only theme (lowercase `cstheme` OOXML quirk) A7: eastAsia-only theme Group B — user override drops theme refs: B1: all 4 themes + Arial B2: customer shape + Arial (Luccas's case) B3: mixed theme+concrete + Arial Group C — no-theme baselines: C1: pure concrete unchanged C2: pure concrete + different concrete C3: empty existing The universal encoder mock mirrors resolveDocxFontFamily's behaviour: any theme ref wins for the primary slot, falling back to concrete ascii/hAnsi/etc. Per-script extras (eastAsiaFontFamily/csFontFamily) attach only when concrete slot value differs from primary fontFamily — same emission rule as the real encoder in super-converter/styles.js. These scenarios are the basis for catching any future drift in the gate logic or merge helper — both the import-preservation path and the user-override path are locked at every shape we ship. --- ...calculateInlineRunPropertiesPlugin.test.js | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js index 6789b1f265..ec7beea5f3 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -1016,4 +1016,248 @@ describe('calculateInlineRunPropertiesPlugin', () => { expect(ff?.cstheme).toBeUndefined(); }); }); + + // Mutation matrix — exhaustively walks the (existing.fontFamily shape, mark-decoded + // shape) cross-product to catch any regression on round-trip integrity. Each row pins + // the expected runProperties.fontFamily shape after the plugin's appendTransaction. + describe('SD-2894 mutation matrix — fontFamily round-trip integrity', () => { + // Theme → CSS map mirrors what `resolveDocxFontFamily` would produce against a + // real theme1.xml. Per-script extras (`eastAsiaFontFamily`, `csFontFamily`) are + // emitted only when the concrete slot value differs from the primary fontFamily — + // matches the real encoder in super-converter/styles.js. + const THEME_TO_CSS = { + majorBidi: 'Calibri Light', + minorBidi: 'Calibri', + majorHAnsi: 'Cambria', + minorHAnsi: 'Calibri', + majorEastAsia: 'SimSun', + minorEastAsia: 'SimSun', + }; + const universalEncoder = (runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + const attrs = {}; + // Mirror resolveDocxFontFamily: any theme ref wins for the primary slot, then fall + // back to concrete ascii/hAnsi/eastAsia/cs. The real resolver also defaults to the + // 'major' theme when nothing else is present, but tests pin explicit shapes. + const themeRef = ff.asciiTheme ?? ff.hAnsiTheme ?? ff.eastAsiaTheme ?? ff.cstheme; + if (themeRef && THEME_TO_CSS[themeRef]) { + attrs.fontFamily = THEME_TO_CSS[themeRef]; + } else if (typeof ff.ascii === 'string') { + attrs.fontFamily = ff.ascii; + } else if (typeof ff.hAnsi === 'string') { + attrs.fontFamily = ff.hAnsi; + } else if (typeof ff.eastAsia === 'string') { + attrs.fontFamily = ff.eastAsia; + } else if (typeof ff.cs === 'string') { + attrs.fontFamily = ff.cs; + } + if (typeof ff.eastAsia === 'string' && ff.eastAsia !== attrs.fontFamily) { + attrs.eastAsiaFontFamily = ff.eastAsia; + } + if (typeof ff.cs === 'string' && ff.cs !== attrs.fontFamily) { + attrs.csFontFamily = ff.cs; + } + return Object.keys(attrs).length ? [{ type: 'textStyle', attrs }] : []; + }; + + const scenarios = [ + // ── Group A: theme preservation (marks re-encode the imported value) ── + { + name: 'A1: all-4-slot theme refs preserved when marks match', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + marksFontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Cambria', + eastAsia: 'SimSun', + cs: 'Calibri Light', + }, + expected: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + }, + { + name: 'A2: SD-2894 customer shape (3 theme slots, no eastAsia) preserved', + existing: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', cs: 'Calibri Light' }, + expected: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + { + name: 'A3: themes on 3 slots + concrete eastAsia preserved (regression case)', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + marksFontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Arial', + cs: 'Calibri Light', + }, + expected: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + }, + { + name: 'A4: ascii-only theme preserved', + existing: { asciiTheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', eastAsia: 'Calibri Light', cs: 'Calibri Light' }, + // hAnsi/eastAsia/cs are mark-derived concrete; only asciiTheme had a theme to preserve. + expected: { + asciiTheme: 'majorBidi', + hAnsi: 'Calibri Light', + eastAsia: 'Calibri Light', + cs: 'Calibri Light', + }, + }, + { + name: 'A5: hAnsi-only theme preserved', + existing: { hAnsiTheme: 'majorHAnsi' }, + marksFontFamily: { ascii: 'Cambria', hAnsi: 'Cambria', eastAsia: 'Cambria', cs: 'Cambria' }, + expected: { + ascii: 'Cambria', + hAnsiTheme: 'majorHAnsi', + eastAsia: 'Cambria', + cs: 'Cambria', + }, + }, + { + name: 'A6: cs-only theme preserved (lowercase `cstheme` OOXML quirk)', + existing: { cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', eastAsia: 'Calibri Light', cs: 'Calibri Light' }, + expected: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Calibri Light', + cstheme: 'majorBidi', + }, + }, + { + name: 'A7: eastAsia-only theme preserved', + existing: { eastAsiaTheme: 'majorEastAsia' }, + marksFontFamily: { ascii: 'SimSun', hAnsi: 'SimSun', eastAsia: 'SimSun', cs: 'SimSun' }, + expected: { + ascii: 'SimSun', + hAnsi: 'SimSun', + eastAsiaTheme: 'majorEastAsia', + cs: 'SimSun', + }, + }, + // ── Group B: user override (marks diverge from existing) ── + { + name: 'B1: all-4-slot theme + user picks Arial → drop all themes', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'B2: customer shape + user picks Arial → drop themes (Luccas case)', + existing: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'B3: mixed theme + concrete eastAsia, user picks Arial → drop everything stale', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + // ── Group C: no-theme baselines (regression sanity) ── + { + name: 'C1: pure concrete unchanged → output stays concrete', + existing: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'C2: pure concrete + user picks Helvetica → output is new concrete', + existing: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + marksFontFamily: { ascii: 'Helvetica', hAnsi: 'Helvetica', eastAsia: 'Helvetica', cs: 'Helvetica' }, + expected: { ascii: 'Helvetica', hAnsi: 'Helvetica', eastAsia: 'Helvetica', cs: 'Helvetica' }, + }, + ]; + + it.each(scenarios)('$name', ({ existing, marksFontFamily, expected }) => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: marksFontFamily, + })); + encodeMarksFromRPrMock.mockImplementation(universalEncoder); + resolveRunPropertiesMock.mockImplementation(() => ({})); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { fontFamily: existing }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual(expected); + }); + + // Special baseline: empty existing (run has no prior fontFamily metadata). marks + // become authoritative; nothing to preserve from existing. + it('C3: empty existing + marks have concrete → marks become the source of truth', () => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + })); + encodeMarksFromRPrMock.mockImplementation(universalEncoder); + resolveRunPropertiesMock.mockImplementation(() => ({})); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { runProperties: null, runPropertiesInlineKeys: null }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual({ + ascii: 'Arial', + hAnsi: 'Arial', + eastAsia: 'Arial', + cs: 'Arial', + }); + }); + }); }); From 851d4e028df4fed51cf31b835cfc18e348655f67 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 13 May 2026 15:08:46 -0300 Subject: [PATCH 5/5] test(style-engine): pin 4-slot theme/concrete dedup; document plugin limitation Addresses two codex review findings on PR #3225: (1) Cascade test coverage: the original SD-2894 bug was that combineRunProperties' slot dedup only handled (ascii, asciiTheme) correctly; hAnsi/eastAsia/cs leaked. Existing cascade tests didn't directly exercise the four-slot behaviour. Adds: - one test per slot for the "theme drops concrete" direction - one test per slot for the "concrete drops theme" direction - an Athenaintelligence-customer-shape test - an export assertion pinning FONT_SLOT_THEME_PAIRS so it stays in sync with the super-editor plugin that re-imports it These prevent any single-slot regression of the original SD-2894 fix. (2) Per-script gate limitation: the plugin's gate + merge pair operate at the fontFamily-key level, not per-slot. A user action that mutates only eastAsiaFontFamily or csFontFamily without changing the primary fontFamily will pass the gate. Documented as an AIDEV-NOTE on mergeFontFamilyPreservingThemeRefs. We accept the trade-off because (a) the toolbar's setFontFamily always changes the primary, (b) no current API or paste path produces this shape against a themed import, (c) the prior attempt at increased precision (f046df9c0) regressed the customer fixture (3 of 65 runs); narrowing to per-slot would re-open that risk without a concrete reproduction. If a real customer scenario surfaces, file a follow-up with the fixture and rework into per-slot decisions then. --- .../style-engine/src/cascade.test.ts | 103 ++++++++++++++++++ .../run/calculateInlineRunPropertiesPlugin.js | 14 +++ 2 files changed, 117 insertions(+) diff --git a/packages/layout-engine/style-engine/src/cascade.test.ts b/packages/layout-engine/style-engine/src/cascade.test.ts index c8f5021f73..9da8ecaa51 100644 --- a/packages/layout-engine/style-engine/src/cascade.test.ts +++ b/packages/layout-engine/style-engine/src/cascade.test.ts @@ -152,6 +152,109 @@ describe('cascade - combineRunProperties', () => { bold: true, }); }); + + // SD-2894: each `` script slot has both a concrete form (`w:ascii`, + // `w:hAnsi`, `w:eastAsia`, `w:cs`) and a theme form (`w:asciiTheme`, + // `w:hAnsiTheme`, `w:eastAsiaTheme`, `w:cstheme` — note `cstheme` lowercase). + // When a higher-priority source supplies one form for a slot, the cascade must + // drop the other form from lower-priority sources, or Word resolves the concrete + // name as an override and defeats per-script theme resolution. + // + // The original SD-2894 bug was that only the (ascii, asciiTheme) pair was + // dropping correctly; hAnsi/eastAsia/cs leaked through. These tests pin the + // full 4-slot dedup so the bug cannot regress for any single slot. + describe('SD-2894 four-slot theme/concrete dedup', () => { + it('drops concrete `ascii` from lower when higher supplies `asciiTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { ascii: 'Calibri' } }, + { fontFamily: { asciiTheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ asciiTheme: 'majorBidi' }); + }); + + it('drops concrete `hAnsi` from lower when higher supplies `hAnsiTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { hAnsi: 'Calibri' } }, + { fontFamily: { hAnsiTheme: 'majorHAnsi' } }, + ]); + expect(result.fontFamily).toEqual({ hAnsiTheme: 'majorHAnsi' }); + }); + + it('drops concrete `eastAsia` from lower when higher supplies `eastAsiaTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { eastAsia: 'SimSun' } }, + { fontFamily: { eastAsiaTheme: 'majorEastAsia' } }, + ]); + expect(result.fontFamily).toEqual({ eastAsiaTheme: 'majorEastAsia' }); + }); + + it('drops concrete `cs` from lower when higher supplies `cstheme` (lowercase)', () => { + const result = combineRunProperties([ + { fontFamily: { cs: 'Arial' } }, + { fontFamily: { cstheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ cstheme: 'majorBidi' }); + }); + + it('drops theme `asciiTheme` from lower when higher supplies concrete `ascii`', () => { + const result = combineRunProperties([ + { fontFamily: { asciiTheme: 'majorBidi' } }, + { fontFamily: { ascii: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ ascii: 'Arial' }); + }); + + it('drops theme `hAnsiTheme` from lower when higher supplies concrete `hAnsi`', () => { + const result = combineRunProperties([ + { fontFamily: { hAnsiTheme: 'majorHAnsi' } }, + { fontFamily: { hAnsi: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ hAnsi: 'Arial' }); + }); + + it('drops theme `eastAsiaTheme` from lower when higher supplies concrete `eastAsia`', () => { + const result = combineRunProperties([ + { fontFamily: { eastAsiaTheme: 'majorEastAsia' } }, + { fontFamily: { eastAsia: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ eastAsia: 'Arial' }); + }); + + it('drops theme `cstheme` from lower when higher supplies concrete `cs`', () => { + const result = combineRunProperties([ + { fontFamily: { cstheme: 'majorBidi' } }, + { fontFamily: { cs: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ cs: 'Arial' }); + }); + + it('Athenaintelligence customer shape: all 4 concretes from defaults dropped by inline themes', () => { + // Mirrors the customer fixture: docDefaults supply concrete fonts (Arial), + // inline rPr supplies theme refs on ascii/hAnsi/cs (no eastAsiaTheme). The + // cascade must keep only the theme refs on those three slots; eastAsia + // concrete from defaults is independent. + const result = combineRunProperties([ + { fontFamily: { ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial', eastAsia: 'Arial' } }, + { fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }); + }); + + it('exports FONT_SLOT_THEME_PAIRS so callers (super-editor plugin) can stay in sync', async () => { + const { FONT_SLOT_THEME_PAIRS } = await import('./cascade.js'); + expect(FONT_SLOT_THEME_PAIRS).toEqual([ + ['ascii', 'asciiTheme'], + ['hAnsi', 'hAnsiTheme'], + ['eastAsia', 'eastAsiaTheme'], + ['cs', 'cstheme'], + ]); + }); + }); }); describe('cascade - combineIndentProperties', () => { diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js index 25747fe71b..157f242fe6 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -44,6 +44,20 @@ export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHype * Without this merge we'd export concrete font names that defeat Word's per-script theme * resolution. * + * AIDEV-NOTE: The gate + merge pair operate at the fontFamily-key level (whole rPr + * fontFamily object), not at the per-slot level. A user action that mutates only a + * per-script mark attr (`eastAsiaFontFamily` or `csFontFamily`) without changing the + * primary `fontFamily` will pass the gate, and this merge will silently restore the + * stale theme on that slot. We accept this trade-off because (a) the toolbar's + * setFontFamily writes all four slots to the same family, so a real user override + * always changes the primary; (b) no current API or paste path produces per-script-only + * mark changes against an imported theme run; (c) a per-slot model adds intricate + * encoder probes that risk regressing the SD-2894 customer fixture again (see commit + * bec7e63ed for the prior per-script regression we just fixed). If a real customer + * reproduction surfaces — e.g. a paste-from-Word flow with per-script fonts being + * silently reverted — file a follow-up ticket with that fixture and rework this helper + * into a per-slot decision then. Codex flagged this on the PR-3225 review. + * * @param {Record|null|undefined} fromMarks * @param {Record|null|undefined} existing * @returns {Record}