Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
We have a lot of content created by Views/CCK. After the 7.x-1.26 update (from 7.x-1.25) all nodes are having issues with the body content being rendered as plain text, with all HTML and images etc stripped. Reverting back to 7.x-1.25 fixes the issue. (Running D7.69 w/ all latest modules)
Comment | File | Size | Author |
---|---|---|---|
#22 | metatag-n3102817-21.patch | 1014 bytes | DamienMcKenna |
| |||
#13 | metatag-n3102817-13.patch | 1.71 KB | DamienMcKenna |
| |||
#13 | metatag-n3102817-13.interdiff.txt | 623 bytes | DamienMcKenna |
#12 | metatag-n3102817-2.patch | 1.7 KB | DamienMcKenna |
Comments
Comment #2
DamienMcKennaThat's unfortunate, I'm sorry to hear it.
Comment #3
slovv CreditAttribution: slovv commentedA similar problem. In my case, formatting of the text except edited by the administrator disappears and only the database rollback helped.
Comment #4
mattltHello,
Just confirming the report, and that reverting to 1.25 brings back html for formatting.
I'm available to test solutions if needed.
Thanks,
•• matt
Comment #5
ron_s CreditAttribution: ron_s commentedI can confirm this is an issue for us too. 1.26 strips all HTML from the summary and body fields. Reverting back to 1.25 fixes the problem.
We're not using Views to render the fields, but we are using Panels/Panelizer.
Comment #6
ron_s CreditAttribution: ron_s commentedAdding panels and panelizer to the tags.
Comment #7
ron_s CreditAttribution: ron_s commentedI see the problem, although I think some code is going to need to be reworked to achieve the original goal.
The issue is with this block of code that was added as part of this patch:
https://www.drupal.org/node/3063056
https://git.drupalcode.org/project/metatag/commit/2c52df6
Specifically, it is this line of code:
https://git.drupalcode.org/project/metatag/blob/7.x-1.x/metatag.inc#L512
When the
if(isset($options['token data']['node'])) {
block of code was added, there is now a call totidyValue
. The problem istidyValue
runs thestrip_tags
function: https://git.drupalcode.org/project/metatag/blob/7.x-1.x/metatag.inc#L229The body is reformatted to strip tags, and then saved back into the token data. Any other rendering after this point is done with the version that has no tags.
I understand what is attempting to be done in Issue #3063056, but the approach being used is not correct. I'm wondering why
hook_metatag_pattern_alter
can't be used to reach the result @tristiangraves is seeking?Comment #8
ron_s CreditAttribution: ron_s commentedGiving this some thought... maybe you'll disagree, but I think the patch should be rolled back, and I don't believe Metatag should be trying to modify token data.
Instead, @tristiangraves should be using an approach like we do for our sites -- use the Smart Trim module (https://www.drupal.org/project/smart_trim) to selectively choose which tags can be passed into [node:summary].
This does two things: ensures the node summary never contains content that should not be present during display (such as embedded media browser content), and cleans up the summary for use with modules like Metatag.
Comment #9
joekings CreditAttribution: joekings commentedI ran into the same issue. We are using silex to add some code to our templates before final rendering and feed it the page html from the node data. Since I updated metatag at the same time as drupal for a while I thought it was the drupal update that had the bug. It wasn't until I updated a test instance that was I able to find the cause of the issue, the updated metatag plugin.
Comment #10
DamienMcKennaComment #11
steveoriolSame problem here, I had to go back to version 1.26
Comment #12
DamienMcKennaCould someone please explain how to replicate the problem?
I don't agree with needing to require another module (Smart Trim) to fix what is a core problem.
How about we change getValue() to put back the original version of the body string after Metatag is done processing the tokens?
Comment #13
DamienMcKennaDoh, sorry about that; this one should work.
Comment #14
steveoriolThe patch #13 fix the issue :-)
But I had to apply it by hand ...
Comment #15
DamienMcKennaThank you steveoriol.
Could someone else please try the fix, just to confirm it resolves the problem? Thanks.
Comment #16
ron_s CreditAttribution: ron_s commentedI will look into it shortly and report my findings.
Comment #17
ron_s CreditAttribution: ron_s commentedHave tested and patch #13 takes care of the issue. Marking as RTBC.
Comment #19
DamienMcKennaGreat!
Let's do the test coverage separately, this should be good for now. Thank you.
Committed.
Comment #20
ron_s CreditAttribution: ron_s commentedI'm sorry, just did a load test with this patch and it needs work. If you'd prefer to have this as a new issue task, let me know.
The problem is this block of code:
On a site with a number of panel panes (using Panelizer), it runs this code each time. Having this patch in place increases the page load time from ~2 seconds to around 28 seconds! Rolling it back fixes the issue.
Comment #21
ron_s CreditAttribution: ron_s commentedOne additional point, I think the cause is the call to
media_wysiwyg_filter
intidyValue
. Running a trace shows calls tomedia_wysiwyg_token_to_markup
are consuming over 20 seconds of time.Comment #22
DamienMcKennaDoes this help?
Comment #23
ron_s CreditAttribution: ron_s commentedGeneral concept makes sense to me, I'll test it out and get back to you.
Comment #24
DamienMcKennaHas anyone tested the patch in #22 yet? Does it help?
Comment #25
ron_s CreditAttribution: ron_s commented@Damien, we're going to test it out, just needs to occur during off-peak hours so we can ensure it doesn't cause any issues in the live environments too. If you can be patient I should be able to let you know by tomorrow.
Comment #26
DamienMcKenna@ron_s: That'd be great, thank you!
Comment #27
ron_s CreditAttribution: ron_s commented@Damien, this patch is certainly better, but it's still a fairly heavy performance hit until pages are cached. This is for a site with tens of thousands of nodes.
The
media_wysiwyg_filter
function is being run for every node loaded to a page, even if it's just a view mode which references a node. This function runsmedia_wysiwyg_token_to_markup
, which is a fairly intensive operation.I can understand why metatag would want it to work this way (need to clean up reference nodes that might be used as related metatags), but it requires a lot of processing. What's most unfortunate is 90% of the nodes on a given page have nothing to do with metatags, yet it's still being forced to run the
media_wysiwyg_filter
function viatidyValue
.Have been watching as new pages are hit before being cached, and we're easily seeing a 15-20% increase in load time for the first rendering.
Comment #28
DamienMcKennaLet's split this off into a separate issue so we don't overload this one with performance discussions that affect a subset of sites: #3107151: Performance problems from entity pre-rendering for summary token
Comment #29
ron_s CreditAttribution: ron_s commentedSounds good, thanks.
Comment #30
AnybodyHi and thank you all very much, is it possible to create a new stable release which fixes the critical issue in the current stable?