Opened 19 years ago
Last modified 11 months ago
#2833 reopened defect (bug)
wpautop breaks style and script tags
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | low |
Severity: | normal | Version: | 2.0.3 |
Component: | Formatting | Keywords: | wpautop has-patch dev-feedback |
Focuses: | Cc: |
Description
When I create a post in which I want to include Javascript or some styles, WordPress 'breaks'when showing those posts, because all newlines in the SCRIPT and STYLE tag are converted into BR tags.
Example:
<style type="text/css> .matt { color: #FFFFFF; } </style>
Becomes:
<style type="text/css><br /> .matt { color: #FFFFFF; }<br /> </style><br />
And:
<script type="text/javascript"><!-- google_ad_client = "xxxxxxxx"; google_ad_width = 120; google_ad_height = 60; google_ad_format = "120x60_as_rimg"; google_cpa_choice = "CAAQ2eOZzgEaCD4zuVkdzt_CKI-293M"; //--></script>
Becomes
<script type="text/javascript"><!--<br /> google_ad_client = "xxxxxxxx";<br /> google_ad_width = 120;<br /> google_ad_height = 60;<br /> google_ad_format = "120x60_as_rimg";<br /> google_cpa_choice = "CAAQ2eOZzgEaCD4zuVkdzt_CKI-293M";<br /> //--></script><br />
This happens because wpautop adds those BR tags to the post. (As it should, just not within STYLE or SCRIPT tags.)
I've made a (temporary?) workaround for this by creating a pre and post event for wpautop, which substitute the necessary newlines by a temporary value in the pre event and placing them back in the post event. Although I think this should be incorporated in wpautop itself.
See also: http://wordpress.org/support/topic/76433 and http://wordpress.org/support/topic/76297
While searching trac I also found ticket #2346, which is about the same problem, but which was for 2.0 and self-closed by the submitter?
P.S. I have TinyMCE turned of.
Attachments (4)
Change History (49)
#2
@
19 years ago
Just looking at your code... the brackets on line 74 look like a mistake to me. As is, it will also match tags like p, pre, tr, li. Once you've dropped the brackets it should be fine.
#5
@
16 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
In 2.7 JavaScript is still damaged by wpautop if it contains empty lines:
test <script language="javascript"> alert( 'test' ); </script> test
will be changed to
<p>test</p> <p><script language="javascript"></p> <p> alert( 'test' );</p> <p></script></p> <p>test</p>
#6
@
16 years ago
It seems that there is already a fix within this plugin that maybe could be used: http://www.automateyourbusiness.com/updates/2007/07/18/javascript-in-wordpress-posts/
#7
@
16 years ago
- Milestone changed from 2.1 to 2.8
confirmed in wp 2.8. and switching back and forth with tinymce breaks it to:
<p><script language="javascript"><!-- alert( 'test' ); // --></script></p>
#8
@
16 years ago
- Cc ShaneF added
- Keywords bg bg needs-patch needs-testing added; bg|has-patch bg|needs-testing removed
After several attempts it seems this has yet to be fixed. The current patch was commit a while back but it still doesn't resolve spaced style or javascript code.
#11
@
16 years ago
I've had a similar issue on one of my plugins, at one point. (As in, things got processed when they really shouldn't.)
To fix, I went something like:
- Replace entire area with a unique place holder
- Process everything
- Replace the unique place holder with its original content
In case it prompts ideas... It would then also fix the SVG problem: #9437
#12
@
16 years ago
- Keywords wpautop break script style br bg bg needs-testing removed
- Milestone changed from 2.8 to Future Release
#15
@
15 years ago
Please close as wontfix. Fixing things will break other things and vice-versa. I do not know a single seriuos developer that wants to touch this.
Best would be to get a decription first what must (not)/should (not)/can (not) be done by wpautop. Then testcases must be written and the the function must be re-worked.
#17
@
12 years ago
Here is what presently happens in trunk:
<p>test</p> <p><script type="text/javascript" language="javascript">// <![CDATA[ alert( 'test' ); // ]]></script></p> <p>test</p>
So seems like the only remaining issue for *this ticket* is incorrectly <p>
'ing the inline script
#19
@
11 years ago
- Keywords has-patch added; needs-patch needs-unit-tests removed
Added a patch that addresses the issues with the contents of 'script' tags being altered. Uses the same method that was used to prevent the contents of 'pre' tags (removing the actual contents of the tag, and replacing with a placeholder). Abstracted the code that replaces the tags into its own helper function, so that there isn't a ton of nearly-identical code inside of wpautop()
Also added a unit test to check the 'script' case - based on the unit tests for 'pre' case.
Tested with groups 2833 (my tests) and 19855 (existing tests for 'pre' tags) and they both passed.
This could conceivably be used in the future for other things that should not have the inner contents modified, such as 'style' tags - maybe others?
This ticket was mentioned in IRC in #wordpress-dev by adamsilverstein. View the logs.
11 years ago
#22
@
9 years ago
As of 4.2.3, we are now avoiding newlines inside of HTML comments and CDATA comments. There are a couple more bugs to work out, but I expect that to be fully working by 4.3.1.
I haven't had time yet to fully review the patch.
#23
@
8 years ago
2833.3.diff is an experiment with this, it works by duplicating wp_replace_in_html_tags()
, and make it work on tag content, too.
This is could be a lot better.
Also, needs more unit tests.
#24
@
8 years ago
Added some tests for the cases given here on 2691 which solves some of the remaining issues raised here.
This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.
8 years ago
#27
@
8 years ago
- Milestone changed from Future Release to 4.8.1
Milestoning for 4.8.1 since this is going to be a particular problem for Text widgets where users paste in script
s that may have double line breaks.
This ticket was mentioned in Slack in #core by pento. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by pento. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by westonruter. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.
7 years ago
#36
follow-up:
↓ 37
@
6 years ago
Since Gutenberg implements a direct JavaScript port of the PHP implementation, it too suffers the same issues described here, which is particularly problematic when considering that it applies autop
to pre-5.0 content for compatibility between block- and non-block content.
A description of the issue and related findings at:
https://github.com/WordPress/gutenberg/issues/9056#issuecomment-485885555
For that reason, I was inspired to pull some of the work here in comment:19 and comment:23 toward a fix in Gutenberg's JavaScript-based implementation:
https://github.com/WordPress/gutenberg/pull/15129
In doing so, I have some remarks on the proposed patches for the PHP implementation:
@pento's patch at attachment:2833.3.diff :
- It did serve to avoid the addition of
br
elements where linebreaks occur inscript
andstyle
tags, but did not help the issue with the addition ofp
elements. - In the
else
path of the patch, there is a reference to an unset$replace
variable. I believe this should be revised to$replace_pairs[ $needle ]
. - I may be mistaken, but I believe the
$i += 2
in theelse
path of the patch should instead be$i++
, as otherwise it can skip the fragments of text which include newlines.
@cmmarslender patch at attachment:2833.2.diff :
- This resolves both issues of
br
elements andp
elements insertion. In fact, it resolves it so well it should also include removal of the previous attempts to fix the issue added in r3965 (it's no longer necessary to create the temporaryWPPreserveNewline
)
Another issue I'd encountered -- not reflected in an patches here -- is that script tags are wrongly wrapped in a p
tag:
wp> wpautop( '<script></script>' ); => string(25) "<p><script></script></p>
As best I can tell, the resolution here is to include script
in the set of $allblocks
, as is done already for style
:
#37
in reply to:
↑ 36
;
follow-up:
↓ 38
@
6 years ago
Replying to aduth:
@cmmarslender patch at attachment:2833.2.diff :
- This resolves both issues of
br
elements andp
elements insertion. In fact, it resolves it so well it should also include removal of the previous attempts to fix the issue added in r3965 (it's no longer necessary to create the temporaryWPPreserveNewline
)
Yeah, the method used in 2833.2.diff (excluding the whole script tag from "processing") should work best.
Another issue I'd encountered -- not reflected in an patches here -- is that script tags are wrongly wrapped in a
p
tag:
...
As best I can tell, the resolution here is to includescript
in the set of$allblocks
That would mean they will always be "pulled out" on a new line, even when inside other tags. That may result in unwanted <p>
after </script>
. Another option would be to remove leading line breaks before <script...>
. This can still bring edge cases but perhaps less often. Not really sure what's the "lesser evil" here :)
Perhaps a more "proper" solution would be to use the method from 2833.2.diff but have two "script tag placeholders":
- One for use when the script tags have line breaks both before and after. This placeholder will be in
allblocks
so it's not wrapped in <p>. - Another for "inline" script tags.
Then the regex that replaces the tags will need to look at "white space" before and after the script tags to determine which placeholder to use.
#38
in reply to:
↑ 37
;
follow-up:
↓ 39
@
6 years ago
Replying to azaozz:
That would mean they will always be "pulled out" on a new line, even when inside other tags. That may result in unwanted
<p>
after</script>
. Another option would be to remove leading line breaks before<script...>
. This can still bring edge cases but perhaps less often. Not really sure what's the "lesser evil" here :)
I wonder if rather than treating these as block-level elements (which they're not) if it would be preferable then to omit them from this `$allblocks` set (so as not to trigger this "pull out" newline behavior), but still apply a similar treatment to them in unwrapping them from `p` ($allblocks
at this line becomes effectively equivalent to $allblocks . '|script|style'
).
Could you share an example markup string where you'd expect the described issue might occur?
#39
in reply to:
↑ 38
@
6 years ago
Replying to aduth:
Could you share an example markup string where you'd expect the described issue might occur?
Replied at https://github.com/WordPress/gutenberg/pull/15129#issuecomment-487174497, maybe better to handle this there :)
The problem is the <script>
tag is allowed/can be added everywhere #text
is allowed, including in "inline" tags like b
, i
, span
, etc. If marked as "block", it will "split" any tag it is contained into.
#40
follow-up:
↓ 41
@
6 years ago
@aduth Does the JavaScript implementation handle comments correctly? see https://core.trac.wordpress.org/ticket/2691
#41
in reply to:
↑ 40
@
6 years ago
Replying to adamsilverstein:
@aduth Does the JavaScript implementation handle comments correctly? see https://core.trac.wordpress.org/ticket/2691
The JavaScript implementation is a line-by-line direct port of the PHP implementation. If it's buggy in PHP, I expect it's similarly buggy in the JavaScript implementation (that said, I've not directly tested the comments behavior).
Ideally the two implementations can be kept well in-sync. The previously-mentioned pull request draws from the (yet-to-be-committed) patches proposed here. I could imagine similarly adapting from #2691 to the JavaScript implementation as well.
This change to wpautop should fix the aforementioned behaviour.
I've tested it on my box, but would like to get some feedback.