Make WordPress Core

Opened 16 months ago

Closed 15 months ago

Last modified 15 months ago

#63632 closed defect (bug) (fixed)

Consider filtered URLs with arg in `wp_customize_url`

Reported by: xipasduarte Owned by: westonruter
Priority: normal Milestone: 6.9
Component: Customize Version: 3.4.1
Severity: minor Keywords: has-patch has-unit-tests
Cc: Focuses: administration

Description

The wp_customize_url function is setting the query string without proper handling of the URL that it receives from admin_url( 'customize.php' ). This leads to issues when the resulting URL contains a query string value. For example:

[...]/customize.php?my_custom_arg?theme=[theme_name]

When this happens it might generate a loop that results in the browser halting the page load with an error.

Attachments (1)

63632.diff​ (2.1 KB ) - added by xipasduarte 16 months ago.

Download all attachments as: .zip

Change History (13)

This ticket was mentioned in ​PR #9102 on ​WordPress/wordpress-develop by ​@xipasduarte.


16 months ago
#1

  • Keywords has-patch has-unit-tests added

Adds theme arg using add_query_arg and takes care to remove any existing aruments with the same key.

Trac ticket: [](https://core.trac.wordpress.org/ticket/63632)

@xipasduarte
16 months ago

#2 follow-up: @westonruter
16 months ago

  • Milestone Awaiting Review → 6.9

@xipasduarte Thanks for reporting this. It makes sense to me. However, do you have an example where the URL has an initial query string? I know it. Would be introduced with filtering the admin URL, but is this an issue with any existing plugins or themes (or sites)?

#3 in reply to: ↑ 2 @xipasduarte
16 months ago

Thanks for the feedback @westonruter. I have an existing case in a plugin my company develops. It is meant as a translations plugin and we inject the param in the URL as a fallback mechanism for language detection. By doing this we broke the customizer link.

This is the plugin: ​https://github.com/26B/unbabble

It's not yet in the repository and we could kind of go around it and edit this link after, in another hook, but it made sense to me have this fixed in core, if any others need this. It seems to be what would be expected.

#4 @SirLouen
16 months ago

  • Version trunk → 3.4.1

#5 @westonruter
16 months ago

  • Owner set to westonruter
  • Status new → accepted

#6 follow-up: @westonruter
15 months ago

  • Keywords changes-requested added

@xipasduarte Thanks for the PR! I added a ​review. Feel free to skip also attaching a diff file to Trac. I personally almost always use GitHub branches when preparing commits for SVN, so keeping all your changes in the PR is just fine.

#7 in reply to: ↑ 6 @xipasduarte
15 months ago

@westonruter Thanks for the review. I believe I addressed everything and also added a few more tests. Take a look and let me know if we need to change anything else.

#8 @westonruter
15 months ago

  • Resolution → fixed
  • Status accepted → closed

In 60499:

Customize: Account for existing query params in the admin URL when wp_customize_url() adds the theme query param.

Query parameters may be inserted in the initial customize.php URL via the admin_url and site_url filters.

Props xipasduarte, westonruter.
Fixes #63632.

#9 @westonruter
15 months ago

  • Keywords dev-feedback changes-requested removed

#11 @westonruter
15 months ago

In 60500:

Customize: Use static closures in tests to reduce memory usage.

Follow-up to [60499].

Props mukesh27.
See #63632.

​@mukesh27 commented on ​PR #9316:


15 months ago
#12

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 60500
GitHub commit: ​9672f54

This PR will be closed.

Note: See TracTickets for help on using tickets.