Opened 4 years ago
Last modified 2 years ago
#36036 assigned defect (bug)
Multiple CDATA regressions in wp-includes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | General | Keywords: | good-first-bug has-patch |
Focuses: | Cc: | ||
PR Number: |
Description
There is invalid XHTML at lines 2084-2097 of wp-includes/theme.php ( https://github.com/WordPress/WordPress/blob/3f3fe5a7ed6473e995f9b96ee4bcf36fe4c71a35/wp-includes/theme.php#L2084-L2097 )
This script block needs to be escaped with /*<![CDATA[*/ ..script content.. /*]]>*/
Otherwise it will cause rendering errors for people serving XHTML as application/xhtml+xml
This was not always like this, as 5 years ago when I made my strict XHTMl theme there were no errors. I recently re-enabled serving as application/xhtml+xml and to my surprise find this regression in the WP codebase.
Attachments (5)
Change History (29)
#2
@
4 years ago
Whoever made that change must not have realized that wp-includes is not only used in the admin interface.
#4
in reply to:
↑ 3
;
follow-up:
↓ 6
@
4 years ago
Replying to SergeyBiryukov:
Also related: #25863, comment:12:ticket:14226.
My issue is with WordPress as a platform, not the pre-installed themes. That comment is for that specific theme. My XHTML5 theme has been working absolutely fine for many years, and it seems like the intention of #18788 was only to remove CDATA in the admin interface. The patch unintentionally affected all theme content though.
#5
in reply to:
↑ 3
@
4 years ago
Replying to SergeyBiryukov:
Also related: #25863, comment:12:ticket:14226.
https://core.trac.wordpress.org/ticket/14226#comment:19
XHTML is fine for other themes or child themes. They just didn't want it in the default theme.
#6
in reply to:
↑ 4
@
4 years ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to Future Release
#7
@
4 years ago
FWIW, this is not a 4.4 regression, wp_customize_support_script()
was added in 3.4 without CDATA: [20893].
#9
@
4 years ago
- Version changed from 3.4 to 4.4.2
Can you merge this and #36037 into a general XHTML regressions bug? I'll submit a patch for the few applicable areas.
#11
@
4 years ago
I can't edit issues so it would be nice if someone with the permissions could change this issue's title to 'multiple CDATA regressions in wp-includes' to better organize this, instead of splitting it up into one ticket per file.
#12
@
4 years ago
I also changed 2 comments that said CDATA is not necessary for HTML5. CDATA sections are not necessary for non-XHTML, and the HTML5 doctype has no bearing on how the browser parses them.
#14
@
4 years ago
- Component changed from Customize to General
- Summary changed from Invalid XHTML at lines 2084-2097 of wp-includes/theme.php to Multiple CDATA regressions in wp-includes
#15
@
4 years ago
For my first patch I implemented some CDATA normalization that directly accessed the contents of a CDATA node, but I decided that this simpler regex on html()
is better. As far as I can tell this is the minimum amount of changes necessary.
Related: [31034] (for #18788).