fix content edit job view html (WP-1004)#619
Conversation
When new React UI was introduced in ff6b525, element .job-wizard was covered with the `<div style="display: none">` element, but this element didn't have the corresponding closing tag.
First condition was forcibly disabled, but the second one was not. This potentially creates stray closing tags when `$needWrapper` is `true`
Fixed unclosed tag inside ContentEditJob view (WP-1004)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PavelLoparev
left a comment
PavelLoparev
left a comment
There was a problem hiding this comment.
Code review findings on the new test file.
| { | ||
| $source = $this->stripNonHtml(file_get_contents(self::VIEW_FILE)); | ||
|
|
||
| $source = preg_replace( |
There was a problem hiding this comment.
Pattern 1 is dead on arrival: this PR removes the only $needWrapper && false block from the template, so this preg_replace will never match after merge. Consider removing it to avoid misleading future readers into thinking such a pattern might still exist in the template.
| public function testHiddenWrapperHasClosingTag(): void | ||
| { | ||
| $source = $this->stripNonHtml(file_get_contents(self::VIEW_FILE)); | ||
| $hiddenOpens = preg_match_all('#<div\s+style\s*=\s*"display:\s*none#i', $source); |
There was a problem hiding this comment.
The test name says "HasClosingTag" but the assertion only checks that at least one <div style="display:none opening tag exists — it never verifies a matching </div> closes it. The test passes trivially even if the closing tag is absent, which is exactly the class of bug this PR fixes.
| ); | ||
| } | ||
|
|
||
| private function resolveTemplate(bool $needWrapper): string |
There was a problem hiding this comment.
resolveTemplate() does not strip or simulate the outer if (!$isBulkSubmitPage) conditional, so the #smartling-app div is always included in the count regardless of context. A regression that only manifests on the bulk-submit screen (where that block is suppressed) would not be caught. Consider adding a testHtmlDivTagsBalanceOnBulkSubmitScreen() case that strips the !$isBulkSubmitPage block.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 participants
No description provided.