Opened 3 weeks ago
Closed 3 weeks ago
#49609 closed enhancement (duplicate)
Code simplification and bug prevention in get_the_content()
| Reported by: | 
  
     | 
      Owned by: | |
|---|---|---|---|
| Milestone: | Priority: | normal | |
| Severity: | minor | Version: | |
| Component: | Posts, Post Types | Keywords: | has-patch close | 
| Focuses: | Cc: | 
Description
In the initial lines of get_the_content():
function get_the_content( $more_link_text = null, $strip_teaser = false, $post = null ) {
	global $page, $more, $preview, $pages, $multipage;
	$_post = get_post( $post );
	if ( ! ( $_post instanceof WP_Post ) ) {
		return '';
	}
	if ( null === $post ) {
		$elements = compact( 'page', 'more', 'preview', 'pages', 'multipage' );
	} else {
		$elements = generate_postdata( $_post );
	}
when the $post argument is null, two cases are possible:
1) get_post() finds a post
2) get_post() doesn't find a post
In case 2) get_the_content() will immediately return, so the remaining lines to set $elements can be simplified for clarity.
$elements = generate_postdata( $_post );
See the attached patch.
Attachments (1)
Change History (6)
    
      
    #1
  
    
        
          
            
 @
            
3 weeks ago
        
    
  
  
  - Milestone Awaiting Review deleted
 - Resolution set to invalid
 - Status changed from new to closed
 
    
      
    #2
  
    
        
          
            
 @
            
3 weeks ago
        
    
  
  
  - Resolution invalid deleted
 - Status changed from closed to reopened
 
Hi ocean90,
Yes, the patch is practically the same but my explanation in #49591 was wrong, so I closed that ticket and submitted a new one - sorry for the confusion!
To rephrase my explanation above, let's focus on the lines:
1	if ( ! ( $_post instanceof WP_Post ) ) {
2		return '';
3	}
4
5	if ( null === $post ) {
6		$elements = compact( 'page', 'more', 'preview', 'pages', 'multipage' );
7	} else {
8		$elements = generate_postdata( $_post );
9	}
When the $post argument is null (line 5) the $_post instance (coming from the global $post) is also available (otherwise get_the_content() returns, line 2), so it's not necessary to build the $elements array with compact(), we can use generate_postdata( $_post ) as well.
Reopening this ticket for further consideration. Thank you!
Andrea
    
      
    #3
  
    
        
          
            
 @
            
3 weeks ago
        
    
  
  
  - Keywords close added
 
If no post is passed to get_the_content() it assumes that it's used in a context where all globals have been set up already. That's why it's using a simple compact() for the existing globals instead of the heavy generate_postdata() function.
    
      
    #4
  
    
        
          
            
 @
            
3 weeks ago
        
    
  
  
  - Summary changed from Minor code cleanup to Code simplification and bug prevention in get_the_content()
 
Only two additional considerations:
First, I realized that "code cleanup" isn't a good choice of words, because it might suggest that the code isn't "clean" right now. Obviously that  wasn't my intention, I should have said "code simplification" (my English is limited, sorry).
Second, get_the_content() assumes that, when the global $post is set, global $page, $more, $preview, $pages, $multipage are also correctly set, but that's not always the case. At least $pages is sometimes not set up correctly, as shown by a report I received a few days ago, see:
I haven't been able to reproduce the problem, but if we replace compact() with generate_postdata(), we guarantee that $elements is set correctly even when the $post argument is null.
To conclude, I think I've shared all the information I have and now I leave the final decision to you. In the meantime, I'm going to edit the ticket summary for clarity.
This seems to be same as you have previously submitted in #49591.
Note that there are two similar named variables,
$_post(with underscore) and$post. Depending on the context they are not the same. This was introduced in [44941].