Url: Fix addQueryArgs and removeQueryArg on URLs with fragments (Word…

…Press#69313) Co-authored-by: davilera <[email protected]> Co-authored-by: paaljoachim <[email protected]> Co-authored-by: gziolo <[email protected]>
carstingaxion · Feb 26, 2025 · 7a4b5aa · 7a4b5aa
1 parent e757d1d
commit 7a4b5aa
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 5 deletions.
diff --git a/packages/url/src/add-query-args.js b/packages/url/src/add-query-args.js
@@ -3,6 +3,7 @@
  */
 import { getQueryArgs } from './get-query-args';
 import { buildQueryString } from './build-query-string';
+import { getFragment } from './get-fragment';
 
 /**
  * Appends arguments as querystring to the provided URL. If the URL already
@@ -26,7 +27,8 @@ export function addQueryArgs( url = '', args ) {
 		return url;
 	}
 
-	let baseUrl = url;
+	const fragment = getFragment( url ) || '';
+	let baseUrl = url.replace( fragment, '' );
 
 	// Determine whether URL already had query arguments.
 	const queryStringIndex = url.indexOf( '?' );
@@ -38,5 +40,5 @@ export function addQueryArgs( url = '', args ) {
 		baseUrl = baseUrl.substr( 0, queryStringIndex );
 	}
 
-	return baseUrl + '?' + buildQueryString( args );
+	return baseUrl + '?' + buildQueryString( args ) + fragment;
 }
diff --git a/packages/url/src/remove-query-args.js b/packages/url/src/remove-query-args.js
@@ -18,14 +18,18 @@ import { buildQueryString } from './build-query-string';
  * @return {string} Updated URL.
  */
 export function removeQueryArgs( url, ...args ) {
+	const fragment = url.replace( /^[^#]*/, '' );
+	url = url.replace( /#.*/, '' );
+
 	const queryStringIndex = url.indexOf( '?' );
 	if ( queryStringIndex === -1 ) {
-		return url;
+		return url + fragment;
 	}
 
 	const query = getQueryArgs( url );
 	const baseURL = url.substr( 0, queryStringIndex );
 	args.forEach( ( arg ) => delete query[ arg ] );
 	const queryString = buildQueryString( query );
-	return queryString ? baseURL + '?' + queryString : baseURL;
+	const updatedUrl = queryString ? baseURL + '?' + queryString : baseURL;
+	return updatedUrl + fragment;
 }
diff --git a/packages/url/src/test/index.js b/packages/url/src/test/index.js
@@ -636,7 +636,7 @@ describe( 'addQueryArgs', () => {
 		);
 	} );
 
-	it( 'should encodes spaces by RFC 3986', () => {
+	it( 'should encode spaces by RFC 3986', () => {
 		const url = 'https://andalouses.example/beach';
 		const args = { activity: 'fun in the sun' };
 
@@ -652,6 +652,15 @@ describe( 'addQueryArgs', () => {
 		expect( addQueryArgs( url, args ) ).toBe( '?sun=true' );
 	} );
 
+	it( 'should add query args before the url fragment', () => {
+		const url = 'https://andalouses.example/beach/#fragment';
+		const args = { sun: 'true' };
+
+		expect( addQueryArgs( url, args ) ).toBe(
+			'https://andalouses.example/beach/?sun=true#fragment'
+		);
+	} );
+
 	it( 'should return URL argument unaffected if no query arguments to append', () => {
 		[ '', 'https://example.com', 'https://example.com?' ].forEach(
 			( url ) => {
@@ -796,6 +805,12 @@ describe( 'getQueryArg', () => {
 		expect( getQueryArg( url, 'baz' ) ).toBeUndefined();
 	} );
 
+	it( 'should not return what looks like a query arg after the url fragment', () => {
+		const url = 'https://andalouses.example/beach#fragment?foo=bar&bar=baz';
+
+		expect( getQueryArg( url, 'foo' ) ).toBeUndefined();
+	} );
+
 	it( 'should get the value of an array query arg', () => {
 		const url = 'https://andalouses.example/beach?foo[]=bar&foo[]=baz';
 
@@ -823,6 +838,12 @@ describe( 'hasQueryArg', () => {
 		expect( hasQueryArg( url, 'baz' ) ).toBeFalsy();
 	} );
 
+	it( 'should return false if the query arg is after url fragment', () => {
+		const url = 'https://andalouses.example/beach#fragment?foo=bar&bar=baz';
+
+		expect( hasQueryArg( url, 'foo' ) ).toBeFalsy();
+	} );
+
 	it( 'should return true for an array query arg', () => {
 		const url = 'https://andalouses.example/beach?foo[]=bar&foo[]=baz';
 
@@ -867,6 +888,23 @@ describe( 'removeQueryArgs', () => {
 			'https://andalouses.example/beach?bar=foobar'
 		);
 	} );
+
+	it( 'should not remove the url fragment', () => {
+		const url =
+			'https://andalouses.example/beach?foo=bar&param=value#fragment';
+
+		expect( removeQueryArgs( url, 'foo' ) ).toEqual(
+			'https://andalouses.example/beach?param=value#fragment'
+		);
+	} );
+
+	it( 'should not remove what looks like a query arg after url fragment', () => {
+		const url = 'https://andalouses.example/beach#fragment?foo=bar';
+
+		expect( removeQueryArgs( url, 'foo' ) ).toEqual(
+			'https://andalouses.example/beach#fragment?foo=bar'
+		);
+	} );
 } );
 
 describe( 'prependHTTP', () => {