#42578 closed defect (bug) (fixed)
PHP functions inside <p> tags creates new <p> tag, breaking the parent tag into two.
Reported by: | karthikeyankc | Owned by: | dd32 |
---|---|---|---|
Milestone: | 4.9.2 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Formatting | Keywords: | revert good-first-bug has-patch fixed-major |
Focuses: | template | Cc: |
Description
Expected behaviour:
<p class="bio-desc"><?php the_author_meta('description'); ?></p>
should render as:
<p class="bio-desc">Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. </p>
What happens:
<p class="bio-desc"></p><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</p> <p></p>
Attachments (4)
Change History (47)
#2
in reply to:
↑ 1
@
7 years ago
Replying to swissspidy:
It's a feature, not a bug. See #40040.
It's a bug not a feature. Wrong, Bad, and Dumb to commit changes to core that break themes everywhere without a word. #40040 was a bad idea.
#3
@
7 years ago
- Keywords close removed
- Milestone changed from Awaiting Review to 4.9.2
I think we should consider reverting this. It doesn't seem like it's forward compatible, as hinted by failing tests after it was introduced. It creates a visual regression that is hard to mitigate without updating every theme that displays author meta, following the example set by default themes.
@johnbillion, what are your thoughts?
#4
@
7 years ago
It looks like it breaks themes in two ways:
Some themes escape the author bio output: https://kathrynwp-automated-transfer-001.blog/2013/09/03/post-1/
As mentioned in the ticket description, some themes wrap the author bio in paragraph tags, leading to double margins/paddings.
#5
@
7 years ago
- Focuses template added
- Keywords needs-patch revert added
Agreed. I didn't consider the consequences of this enough.
#6
@
7 years ago
The aim of #40040 was to make output consistent by running wptexturize, convert_chars, wpautop and shortcode_unautop on the author description. This is what's done to post content, category descriptions etc.
#40040 hasn't been popular with some theme authors because the change may result in nested <p>
tags if <p>
tags are already used in author description markup.
I agree that this needed more testing.
#8
follow-up:
↓ 9
@
7 years ago
Hi, I would like to try this one as my first patch. As I understand this was to just revert 40040?
I reverted the changes and it seems to be working as expected now. If I'm missing something please let me know.
Thank you
#9
in reply to:
↑ 8
@
7 years ago
Replying to 0x6f0:
If I'm missing something please let me know.
The reason for this ticket is missing: The 'get_the_author_description' in filter array.
To fix this ticket, it is actually enough to just remove 'get_the_author_description', no other changes necessary.
This ticket was mentioned in Slack in #forums by zoonini. View the logs.
7 years ago
#11
@
7 years ago
Thank you @Ov3rfly
Sorry, my test case was off. I should've followed exactly what @karthikeyankc had but I changed it to
<p class="bio-desc"><?php the_author_meta('user_description'); ?></p>
which doesn't behave the same. So I thought I had it fixed.
It's working now though, sorry again.
This ticket was mentioned in Slack in #themereview by iceable. View the logs.
7 years ago
#15
@
7 years ago
Are we looking at a likely chance of reverting this change?
Since this was implimented I have been undertaking the grueling tasks of trying to find all affected authors and notify them when there themes usage has been affected. There are several hundred themes affected in the directory including at least 1 core theme.
If this is to be reverted I can stop doing that but if not then I'll need to confine reaching out to all those authors and informing them.
#17
in reply to:
↑ 16
@
7 years ago
Replying to swissspidy:
@williampatton yes, this is currently slated for 4.9.2
Awesome, thanks for update :)
I seen the 4.9.2 tagging but know that sometimes tagged items get pushed back, seeing the patch tag was removed got me worried :p
This ticket was mentioned in Slack in #forums by zoonini. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
7 years ago
#22
follow-up:
↓ 23
@
7 years ago
I know the choice seems to have been made, I hope I can influence the thought process a little here.
Right now, we are discussing reverting the changes because, basically, some theme developers are frustrated. And I get that. I make themes.
But, as of right now, a handful of theme authors, myself included have already fixed our sites. Its not hard to change a <p> to a <div>, seriously, and if you revert this, I'm going to have to go out and ask my clients again to pay me to revert my changes. (Or, I'm hoping, maybe it won't be required.)
@swissspidy was right, this is a feature. One that was stupid late to the party. One I feel should be kept.
When I make a theme for a client, my goal is to deliver it and them not require me to be on call to make updates. I have a client that has 400 authors. They saw some examples of author info boxes that were richly detailed and they asked me why they can't even get a line break when desired.
I think more people, in the past and moving forward, are more annoyed by the lack of a feature like basic formatting in a user-changeable box, than the group of theme devs who are frustrated by this.
I fully 100% concede that maybe this change wasn't highlighted enough, or promoted enough, but I don't think we should take a step backwards when we know that the audience we're making WordPress for are publishers, not developers. This feature makes it easier for a publisher to make a change without a developer to help them.
This would be sad change to revert.
Further, Matt's keynote at WCUS gave me the impression that we want more and more of this user-friendly formatting in the future. Also, 4.8's WYSIWYG widgets are another example.
I think the theme devs should pause, appreciate the change, and accept that every now and then an update to core might be good for the future of WP, even if it causes a minor headache today.
My 2¢.
#23
in reply to:
↑ 22
@
7 years ago
Replying to jeremyescott:
Right now, we are discussing reverting the changes because, basically, some theme developers are frustrated. And I get that. I make themes.
But, as of right now, a handful of theme authors, myself included have already fixed our sites. Its not hard to change a <p> to a <div>, seriously, and if you revert this, I'm going to have to go out and ask my clients again to pay me to revert my changes. (Or, I'm hoping, maybe it won't be required.)
@swissspidy was right, this is a feature. One that was stupid late to the party. One I feel should be kept.
I agree, reverting will penalise theme authors who have already updated their themes.
Between the choices of retaining or reverting, I think reverting is the lesser evil as more themes are likely to be broken at this time than fixed.
For the themes you have fixed, I suggest adding the filters within functions.php
as a way to maintain the 4.9 functionality.
- the authors archive description can be filtered on
get_the_author_description
- the post type archive description will need to be filtered on
get_the_archive_description
#24
@
7 years ago
@peterwilsoncc: To recap, you're saying the lesser of two evils is to break themes again?
I can think of many cases where I wouldn't have been against a revert, like during development, during beta testing, even a week after release. But its been 8 weeks. For every stubborn theme dev who hasn't fixed their theme by now, there are many who have. Because its an easy fix. Either change a <p> to a <div> or change the escape from esc_html to wp_kses or something like it. But now you're gonna punish all the people who made that fix because some people didn't want to.
Its foolish to try and measure those who have fixed it against those who haven't and somehow decide who loses. But I will say, the businesses that make up the ecosystem around WP have already fixed it. The premium theme devs have already fixed it. They were fixing it on day 1, or even finding it during beta. They shouldn't get preferential treatment, but you know who should? Our end-users, the publishers.
The reason a lot of people may not have reported it as a bug during dev and beta is simply that it fits the mission of WordPress: to make publishing easier. That line break... its important to an unskilled publisher who is wondering why they can't make a paragraph in their bio. That line break matters. And you gave it to them. Now you want to take it away.
Reverting will fix an unknown number of broken sites while breaking an unknown number of fixed sites while taking away a feature that democratized publishing.
This is a sticky situation, I get it. But for where we are now, the failure here is no longer the half-baked rollout of a feature, per se, it is the lack of communication that got us into this mess and the heel-dragging that sees it still unresolved 8 weeks later. It is hugely irresponsibly late to revert this.
And at this point, we should be considering the impact on our publishers, who've wanted something like this for a long time, and the impact on the professionals whose careers forced them to fix this ASAP instead of waiting 8 weeks for a revert. If this hadn't been committed or if it wasn't out in the wild for 8 weeks already, I'd agree with excluding or reverting it, but since it was, and since tons of theme devs have already fixed their themes/sites, if it comes down to should we fix what is broken thus breaking what was fixed, or leave what is broken broken, shouldn't the mission of making publishing easier be considered when tipping the scales? I think so.
I'm speaking for hard workers out there who just dealt with this problem and moved on. Don't punish them. And I'm speaking for the publishers who've wanted a line break in their bio for years. Don't take that away from them.
#25
@
7 years ago
Between the choices of retaining or reverting, I think reverting is the lesser evil as more themes are likely to be broken at this time than fixed.
Agree. Although the changes was good but it was breaking change. It went to core pretty silently without even mentioning the dev notes. Any breaking changes should be pushed to core with larger visibility I believe. Lets revert this in this version. In the next major release we can add it again with proper approach.
#26
@
7 years ago
In the next major release we can add it again with proper approach.
A proper approach would include the_author_meta( 'description' )
changes in Twenty Sixteen, Twenty Fifteen, Twenty Thirteen, Twenty Twelve and all related child themes and all other themes which are based on these official WordPress themes... which is impossible.
#27
follow-up:
↓ 32
@
7 years ago
I have to kind-of agree with not reverting this.
- The damage is done, and we didn't fix it within a reasonable timeframe (IMHO 8 weeks is not reasonable) which has caused themes to update for it.
- The user expectation of being able to add line breaks, and in the future, rich-text is made harder by reverting
- Reverting this in 4.9.x, and adding it back in 5.0~5.5 isn't a viable method forward IMHO, all it does is delay breakages, which have already been broken for weeks.
So while I agree reverting was the best method forward 6-8 weeks ago, I'm not sure it's still the best way forward.
I'd be interested in seeing more data about how many themes are actually broken by this now - have the majority of WordPress.org-hosted themes updated? Did they break in the first place?
Does anyone have a copy of all themes locally they could scan through?
#28
@
7 years ago
I'd be interested in seeing more data about how many themes are actually broken by this now
Twenty Sixteen: 700.000+
Twenty Fifteen: 400.000+
Twenty Thirteen: 100.000+
Twenty Twelve: 300.000+
Child themes of above: uncountable
Custom themes based on these official WordPress themes: uncountable
Reverting .. and adding it back ..
Revert it, do not add it back.
#29
@
7 years ago
@Ov3rfly: All the core theme installs can be fixed by changing 1 line on four themes. I'll volunteer to do if core team commits to not reverting. It will be easy.
Revert it, do not add it back.
So never give publishers rich text options for their bios? That doesn't fit our mission of democratizing publishing. Even if this is reverted, a future with rich text/wysiwyg for all user input fields should be in our heads. Hence why this will be changed one day.
do not add it back.
Why hold WordPress back forever because of a short-term frustration? Fix the affected themes and move on. Its a worthwhile change for our end-user publishers!
#30
@
7 years ago
Here are few themes from the Popular page in the directory where themes are not updated for the fix.
- Customizr - 100k
- ColorMag - 90k
- Hueman - 80k
- Hestia - 60k
- Astra - 20k
This is not only about 4 default themes. We should consider thousands of themes in the ecosystem. Back Compat is the motto of WordPress. Or if any breaking changes then developers should be well informed prior pushing to the core.
As of Theme Review Team, we have not asked theme authors to fix the author description as we were hoping this will be reverted. Even recently live themes have this issue.
#31
@
7 years ago
Fix the affected themes and move on. Its a worthwhile change for our end-user publishers!
It is not possible to fix all affected themes.
This "feature" is clearly "plugin-land", plugins for this feature exist since years, no need to mess up the core.
One more affected example, not a theme, but still:
- JetPack: 4+ Million
(modules/theme-tools/content-options/author-bio.php)
#32
in reply to:
↑ 27
@
7 years ago
Replying to dd32:
I have to kind-of agree with not reverting this.
This was written without realising that the Bundled themes were also affected by this, which downright sucks :)
It's going to mean that some people who have done work to fix it will have only done it for a short-term relief, but hopefully changing it back doesn't affect them too much.
I'm going to backtrack and agree with reverting, with the intention that this is brought back in 5.0 in a backwards-compatible manner.
That may mean going against #8776 which deprecated multiple functions in preference for the_author_meta( 'description' )
and instead deprecating the_author_meta( 'description' )
in preference for a new similarly named the_new_pretty_author_description()
type function which does output <p>
tags deliberately from inception, and maybe even adds WYSIWYG editing to that field.
#34
@
7 years ago
Let's get a new function
Exactly.
Revert this breaking change (and that reverting it has taken this long is the problem with all forms of "decisions by committee"), and then add the ability for publishers and theme builders to override the default behavior by opting in to new behavior which leaves the old default behavior alone if the new one is not intentionally used.
That's what should have been done in the first place. That's what should be done now.
And BTW as a "hard working" developer, I say this as someone who had to modify things on sites I manage when they broke in 4.9 and will have to re-modify again when this poorly thought-out change is finally reverted.
This ticket was mentioned in Slack in #themereview by rabmalin. View the logs.
7 years ago
#36
@
7 years ago
- Keywords needs-refresh added
- Owner set to peterwilsoncc
- Status changed from new to accepted
Reviewing blame logs:
- the post type archive portion of
get_the_archive_description()
wasn't added until 4.9, [40976], it's always been formatted - 42578.3.diff will need a refresh to remove the partial revert of [41232].
#39
@
7 years ago
- Keywords fixed-major added; commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for back-porting.
It's a feature, not a bug. See #40040.