wp_nav_menu suffix in Custom links to remote websites

[stouch]

Bug Description

This plugin adds a suffix /amp to my custom links to remote websites in my footer menu (wp_nav_menu).

My blog is under a subpath (/blog) so I was able to manually remove this suffix (and keep it only for my blog links) with :

		if(strstr($parsed_url['path'], 'blog')){
			$parsed_url['path'] .= user_trailingslashit( amp_get_slug(), 'amp' );
		}

in amp/src/PairedUrl.php.

Expected Behaviour

The suffix must no be added to custom links in wp menus.

Screenshots

No response

PHP Version

7.3

Plugin Version

latest

AMP plugin template mode

Transitional

WordPress Version

No response

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

No response

Browser(s) Affected

No response

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

[westonruter]

The plugin should only be adding the AMP endpoint to links on the same domain as the WP install. Are you saying it is adding the link to other parts of your site which are not part of the WordPress subdirectory install? What does home_url() return for you?

The logic for determining whether a given link should get the AMP endpoint is as follows:

amp-wp/includes/sanitizers/class-amp-link-sanitizer.php

Lines 280 to 319 in 092b4b8

	/**
	* Determine whether a URL is for the frontend.
	*
	* @param string $url URL.
	* @return bool Whether it is a frontend URL.
	*/
	public function is_frontend_url( $url ) {
	$parsed_url = wp_parse_url( $url );
	if ( ! empty( $parsed_url['scheme'] ) && ! in_array( strtolower( $parsed_url['scheme'] ), [ 'http', 'https' ], true ) ) {
	return false;
	}
	// Skip adding query var to links on other URLs.
	if ( ! empty( $parsed_url['host'] ) && $this->home_host !== $parsed_url['host'] ) {
	return false;
	}
	// Skip adding query var to PHP files (e.g. wp-login.php).
	if ( ! empty( $parsed_url['path'] ) && preg_match( '/\.php$/', $parsed_url['path'] ) ) {
	return false;
	}
	// Skip adding query var to feed URLs.
	if ( ! empty( $parsed_url['path'] ) && preg_match( ':/feed/(\w+/)?$:', $parsed_url['path'] ) ) {
	return false;
	}
	// Skip adding query var to the admin.
	if ( ! empty( $parsed_url['path'] ) && false !== strpos( $parsed_url['path'], $this->admin_path ) ) {
	return false;
	}
	// Skip adding query var to content links (e.g. images).
	if ( ! empty( $parsed_url['path'] ) && false !== strpos( $parsed_url['path'], $this->content_path ) ) {
	return false;
	}
	return true;
	}

It seems it is not taking into account the path of the home_url and is only considering the host.

Nevertheless, there is a filter you should be able to use to prevent this from happening in the immediate term. Something like this should do it:

add_filter( 
  'amp_to_amp_linking_element_excluded', 
  static function( $excluded, $url ) {
    if ( ! str_contains( $url, '/blog' ) ) {
      $excluded = true;
    }
    return $excluded;
  }, 
  10, 
  2
);

[stouch]

Thanks.
Actually, We have a website under ourdomain.com/app and we have our blog under /blog. And we have some wp_nav_menu custom links that point to ourdomain.com/app/xxx etc.
I already had an issue with the fact that this plugin does not handle well the case when wp is hosted under a subdirectory :
#6399

Thank you for your add_filter solution (I did not tested it yet)

[westonruter]

@thelovekesh Would you update the logic in is_frontend_url() to verify that the provided $url has the same path prefix as the home_url()? We an add a home_path member variable where we are currently storing the home_host:

amp-wp/includes/sanitizers/class-amp-link-sanitizer.php

Line 82 in 092b4b8

$this->home_host = wp_parse_url( home_url(), PHP_URL_HOST );

$parsed_home     = wp_parse_url( home_url() );
$this->home_host = $parsed_home['host'];
$this->home_path = $parsed_home['path'];

[westonruter]

@stouch The fix has been merged, and there is a pre-release production build you can test: #7330 (comment)

[pavanpatil1]

QA Passed ✅

Cross-checked the issue, Fix is working as expected. Now, the suffix(/amp) for the custom link is not visible.

Update
Steps to test:

1. On the multisite network, Create two sub-directory site named /app, /blogs and enable the AMP plugin on any one of these sites and enable the transitional mode.
2. Now add the custom link from the non-amp site to the AMP site.
3. Once done, validate the Custom link from the AMP site. The result should be as follows:

Before:

After Fix: