Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#2406 closed defect (bug) (invalid)

add_query_arg() does not return Strict XHTML links.

Reported by: majelbstoat Owned by: markjaquith
Priority: normal Milestone:
Component: Administration Version: 2.0.1
Severity: normal Keywords: add_query_arg ampersand xhtml strict bg|has-patch
Cc:

Description

add_query_arg() is a useful general purpose function that can be employed to extend a query string if it already exists. However, the appending of new variables is done with '&' rather than '&', which means that links returned are not XHTML Strict 1.0 or XHTML 1.1 compliant. This is not important for wp-admin pages, but causes problems on site pages. Parsing errors in Firefox and possible more browsers cause the site not to be visible, returning only an XML error. The following patch alters add_query_arg() to append using the correct '&' connector, which would greatly enhance its usability for plugins that wish to work with links.

Attachments (1)

add valid ampersands.diff (508 bytes) - added by majelbstoat 7 years ago.
add & instead of &

Download all attachments as: .zip

Change History (11)

add & instead of &

An alternative solution would be to add a 'strict = false' final argument which would determine whether to output & or &. This might necessitate a refactoring of the argument handling code at the top.

  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

+1

XHTML validity is one of WordPress' big things. Don't have time to give this a thorough look now, but I'll return to it.

Might break if we use it in HTTP, for example. I recommend adding a parameter which is on by default.

comment:4   ryan7 years ago

Or leave it alone and run the output through urlencode() if so needed.

I assume you mean htmlentities ;)

er, yeah.

Yeah, that's what I ended up doing.

And will using & break HTML?

No, that's not what I said. It will break if used in HTTP headers sent with a header() call.

comment:9   matt6 years ago

  • Resolution set to invalid
  • Status changed from assigned to closed

Yes we should do this on a different level, so we don't break redirects.

  • Milestone 2.1 deleted
Note: See TracTickets for help on using tickets.