Fix extra <div> tags accumulating on 'Edit as New' for HTML messages - #10283
Fix extra <div> tags accumulating on 'Edit as New' for HTML messages#10283dodjango wants to merge 3 commits into
Conversation
…oundcube#9919) prepare_html_body() only skipped the body container wrapper for MODE_DRAFT. MODE_EDIT ('Edit as New') fell through to the else branch and got wrapped in a <div id="editbody..."> container by wash_html(). Since Edit-as-New reloads a full message for re-sending (like a draft), that wrapper was saved on send and a new one was added on every edit/send cycle, producing nested <div> buildup. Treat MODE_EDIT like MODE_DRAFT so no container wrapper is added, matching how the two modes are already grouped throughout compose.php. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ibute (roundcube#9919) The previous fix set ignore_elements => ['body'] for MODE_DRAFT/MODE_EDIT in prepare_html_body(), but wash_html() unconditionally registers a callback for the 'body' tag. A registered callback takes precedence over ignore_elements in rcube_washtml::dumpHtml(), so the callback still ran and wrapped the body's style attribute (added by send.php's default_font/default_font_size wrapping on every send) into a new <div> on every edit/send cycle - just without an id this time. Add a skip_washer_body_callback option, mirroring the existing skip_washer_form_callback/skip_washer_style_callback pattern, and set it for MODE_DRAFT/MODE_EDIT so ignore_elements actually takes effect and the <body> tag (and its style attribute) is dropped entirely, keeping only its content. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
02c95ef to
928760d
Compare
…oundcube#9919) Draft mode shares the same no-container code path as edit mode, so a saved draft that is reopened repeatedly is susceptible to the same <div> buildup. Forward mode must keep wrapping the body in a container, so guard that the fix does not regress style isolation for forwards.
|
Friendly ping — this PR is ready for review whenever a maintainer has a moment. 🙏 Two updates since it was opened: Rebased onto current Added two test cases so the coverage matches what the test plan above describes: A note on the previously red check: the single failing job (Linux / PHP 8.3, Message Rendering workflow) was a transient infrastructure failure unrelated to this change — Happy to address any feedback. Thanks! |
1 participant
Summary
Fixes #9919 — repeatedly using "Edit as New" on an HTML message caused extra
<div>wrappers to accumulate in the message body on every cycle. This turned out to have two independent causes, both fixed here:prepare_html_body()only skipped the container-id wrapper (<div id="editbody...">) forMODE_DRAFT, notMODE_EDIT("Edit as New"). Since Edit-as-New reloads/resends the message like a draft, the wrapper was saved and re-added on every cycle.ignore_elements => ['body']forMODE_DRAFT/MODE_EDIThad no effect, becausewash_html()unconditionally registers a callback for thebodytag, and a registered callback takes precedence overignore_elementsinrcube_washtml::dumpHtml(). That callback unconditionally wraps the body (and itsstyleattribute, e.g. thefont-size/font-familyadded bysend.phpon every send) in a new<div>— so the accumulation continued even after fix Change version text to reflect new versioning system #1, just without anid. Added askip_washer_body_callbackoption (mirroring the existingskip_washer_form_callback/skip_washer_style_callbackpattern) and set it forMODE_DRAFT/MODE_EDITso the body tag and its attributes are dropped entirely, keeping only its content.Test plan
tests/Actions/Mail/ComposeTest.phpcoveringMODE_EDIT,MODE_DRAFT(both must not wrap the body, including when the stored<body>tag carries a style attribute), and regression coverage forMODE_REPLY/MODE_FORWARD(must still wrap in a container)ImapTest::test_sort_folder_list,ImageTest,SearchTest,InstallTest::test_check_mime_extensions) were confirmed to pre-exist identically on unpatchedupstream/masterin a side-by-side worktree comparison — no regressions introduced<div>wrappers (with growing ids, then style-only once fix Change version text to reflect new versioning system #1 alone was applied); patched instance shows no wrapper at all, regardless of cycle count<div id="replybody...">, unaffected), Forward (still wraps in<div id="forwardbody...">, unaffected)