WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#50585 closed defect (bug) (fixed)

The Return Value of esc url raw() can be more accurate in its description

Reported by: stevenlinx Owned by: SergeyBiryukov
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Formatting Keywords: has-patch
Focuses: docs Cc:

Description

"The cleaned $url after the 'clean_url' filter is applied. An empty string is returned if $url specifies a protocol other than those in $protocols, or if $url contains an empty string."

Codex:
https://codex.wordpress.org/Function_Reference/esc_url_raw

DevHub:
https://developer.wordpress.org/reference/functions/esc_url_raw/

Attachments (3)

50585.diff (829 bytes) - added by stevenlinx 5 months ago.
50585.2.diff (816 bytes) - added by audrasjb 4 weeks ago.
Small refresh against trunk
50585.3.diff (1.2 KB) - added by helen 4 weeks ago.

Download all attachments as: .zip

Change History (10)

@stevenlinx
5 months ago

#1 @stevenlinx
5 months ago

  • Keywords has-patch added; needs-patch removed

#2 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.6

#3 @SergeyBiryukov
4 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@audrasjb
4 weeks ago

Small refresh against trunk

@helen
4 weeks ago

#4 @helen
4 weeks ago

Apologies for getting so nitpicky on docs which I'm sure are helpful overall - to help me understand how best to evalute these, is the goal to give users a level of documentation that doesn't require needing to look at the codebase? In this instance I think it would be better to point to esc_url() and update the docs there.

See 50585.3.diff for what I think this could look like.

#5 @stevenlinx
4 weeks ago

"...is the goal to give users a level of documentation that doesn't require needing to look at the codebase?"
yes

1.)
I would still prefer having a more descriptive inline doc for both esc_url() and esc_url_raw(), if possible. This is primarily for DevHub and not source code.

IMO (may be just me and not everyone), having only the inline doc only on esc_url(), but not esc_url_raw() feels unsatisfactory due to DevHub parameters info are parsed from the source code inline doc.

When reading the source code file, esc_url_raw() is only a wrapper for esc_url() ; the code position for esc_url_raw() is only below esc_url(), so the reader can just scroll up.

However, on the DevHub page, the visitors would have to load another web page just to get the basic parameter info.

2.)
Regarding another bit of Return parameter info:
"An empty string is returned if $url specifies a protocol other than those in $protocols, or if $url contains an empty string."

I assume this should be included?

However, in patch 3, this bit of information would be dropped from both esc_url() and esc_url_raw().

In patch 3, the inline doc tells people to look at esc_url() for Return parameter information.

https://developer.wordpress.org/reference/functions/esc_url/

If you visit the esc_url() DevHub page and scroll down to the comments section, another commenter said
"If the URI protocol is not one of the allowed protocols, the result of esc_url() is an empty string." (and this commenter missed another piece "An empty string is returned if $url contains an empty string")

So why does this commenter need to mention this specific bit of information in a comment? because the docs are unsatisfactory and incomplete, which further justifies that this type of info shouldn't only be in the source code or the comments.

Last edited 4 weeks ago by stevenlinx (previous) (diff)

#6 @SergeyBiryukov
4 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 49512:

Docs: Improve return value description for esc_url().

Add a reference to esc_url() from esc_url_raw() return value description.

Props stevenlinx, audrasjb, helen.
Fixes #50585.

#7 @SergeyBiryukov
4 weeks ago

In 49513:

Docs: Add a @see reference to esc_url() from esc_url_raw().

Follow-up to [49512].

See #50585.

Note: See TracTickets for help on using tickets.