Opened 12 years ago
Closed 9 years ago
#21282 closed enhancement (maybelater)
Introduce wp_header() pluggable function
Reported by: | sirzooro | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.4.1 |
Component: | Bootstrap/Load | Keywords: | has-patch 2nd-opinion needs-refresh |
Focuses: | Cc: |
Description
I will be nice to have pluggable function wrapper for PHP header()
function - this will allow to unit test HTTP headers sent by WordPress. Attached patch adds it, and replaces all existing calls to header()
with it.
Question: some of exiting calls had errors silenced using @
, some not. In my code I added @
. Should I keep it, remove it or maybe add extra param to function to conditionally silence errors?
Attachments (1)
Change History (7)
#2
@
12 years ago
When header()
function is used in tested code, PHPUnit will report error in tests because of "Cannot modify header information - headers already sent" warning. Therefore header()
function in tested code has to be either prefixed with @
or stubbed. My patch allows to use the latter approach.
Regarding pluggable functions - I can use another approach, e.g. call wp_header
action from wp_header()
function, and hook a function to it.
#3
@
12 years ago
As this patch stands right now, it will silence any errors with production site code sending back headers, not just unit tests, so I don't think this should be applied as is.
At least in all the situations that involve redirect headers, those *should* be calling wp_redirect()
, and wp_redirect()
is also already unit testable. So any header()
calls used for redirect should be changed to use that, especially since plugins using filters on redirects are expecting WordPress to use that call before any redirect.
I'm still not sure how to handle anywhere else that uses any other type of header though, but I also don't think there's one blanket solution for every method that calls header()
. In many cases, those are the primary functionality of those functions, and they need to be tested that the headers are correctly generated (even if they aren't sent).
We already have a wp_redirect() function, which should be used instead of
header( "Location: ..." )
.I don't see how changing the other headers, such as "Content Type" would help with testing.
Also, I personally don't like pluggable functions.