HTML fragment responses are erroneously converted into incomplete AMP documents

[tryazi]

Update from @westonruter:

I've created a sample plugin that can be used to easily reproduce the issue: https://gist.github.com/westonruter/ca17e51c1fe7fe25efc95f04e98b3fbc

After activating the plugin and enabling native/paired mode, going to a URL such as https://example.com/?test_fragment&amp will show:

The problems with this are:

1. An HTML fragment is converted into an HTML document, unexpectedly.
2. The character encoding is all messed up. Potentially because it lacked a <meta charset=utf-8> to begin with.
3. The resulting AMP document is not even valid. This is likely due to the lack of the wp_head action not being run.

We should at the very least prevent starting the post-processor when the response does not contain <html. This can be done by extending the logic here:

amp-wp/includes/class-amp-theme-support.php

Lines 1522 to 1530 in c92e129

	/*
	* Check if the response starts with HTML markup.
	* Without this check, JSON responses will be erroneously corrupted,
	* being wrapped in HTML documents.
	*/
	if ( '<' !== substr( ltrim( $response ), 0, 1 ) ) {
	return $response;
	}

However, we also need to validate the other two points to see if there isn't some bug to fix.

Original description is as follows.

---

Hello there
I'm using your plugin. There is nothing wrong. But I see a character error in the search bar on the main page. In fact, everything was good for a few days. I didn't understand why.

When you put your plugin inactive, everything returns to normal. I've looked at the link below, but there are already these codes.

amp-wp/includes/class-amp-theme-support.php

Line 192 in e60cb15

// @todo Add character conversion.

Example
https://www.odevbul.org (search pls)

[westonruter]

What is the DB_CHARSET on your site? If your database isn't utf-8 then that would explain it.

[tryazi]

I'm using Utf-8. I couldn't make sense for a few days. He gave a DB error. It happend like that. It improves if I pass the plug-in

[westonruter]

Regarding the line of code you linked to:

amp-wp/includes/class-amp-theme-support.php

Line 192 in e60cb15

// @todo Add character conversion.

This is only relevant for sites that are not already served in UTF-8. For that you can refer to #855. But since your site is already in UTF-8, I don't know why you're seeing characters that are malformed.

[westonruter]

Looking at the network log, I can see that the auto-completions are being populated via XHRs with URLs like: https://www.odevbul.org/?livesearch=used&s=test

If you look at that URL, you'll see that this URL unexpectedly has the amp attribute on the html element.

[tryazi]

I think I have no choice but to disable live search. That's what I did. The upcoming updates will probably resolve.

[westonruter]

What plugin are you using for live search?

[westonruter]

And how is your AMP plugin configured? Please share a screenshot of your AMP settings screen.

[tryazi]

This plugin is not a js file inside the theme.

[tryazi]

If I choose topics and articles from the options, this problem still continues.

[westonruter]

@tryazi What about the templates? Are you excluding certain templates from being served as AMP?

What are you using for the live search?

[tryazi]

Yes, I tried the templates. Even these templates are marked as mauel amp disable. I added my .js file. It's not important.

Another problem is the inconvenience of the use of accordion. I'm writing content into tags. However, when the page is published, the label is closed.

these problems do not want to keep you. You may have bigger problems.

/***
@title:
Live Search

@version:
2.0

@author:
Andreas Lagerkvist

@date:
2008-08-31

@url:
http://andreaslagerkvist.com/jquery/live-search/

@license:
http://creativecommons.org/licenses/by/3.0/

@copyright:
2008 Andreas Lagerkvist (andreaslagerkvist.com)

***/
jQuery.fn.liveSearch = function (conf) {
	var config = jQuery.extend({
		url:			'/search-results.php?q=', 
		id:				'jquery-live-search', 
		duration:		400, 
		typeDelay:		200,
		loadingClass:	'loading', 
		onSlideUp:		function () {}, 
		uptadePosition:	false
	}, conf);

	var liveSearch	= jQuery('#' + config.id);

	// Create live-search if it doesn't exist
	if (!liveSearch.length) {
		liveSearch = jQuery('<div id="' + config.id + '"></div>')
						.appendTo(document.body)
						.hide()
						.slideUp(0);

		// Close live-search when clicking outside it
		jQuery(document.body).click(function(event) {
			var clicked = jQuery(event.target);

			if (!(clicked.is('#' + config.id) || clicked.parents('#' + config.id).length || clicked.is('input'))) {
				liveSearch.slideUp(config.duration, function () {
					config.onSlideUp();
				});
			}
		});
	}

	return this.each(function () {
		var input							= jQuery(this).attr('autocomplete', 'off');
		var liveSearchPaddingBorderHoriz	= parseInt(liveSearch.css('paddingLeft'), 10) + parseInt(liveSearch.css('paddingRight'), 10) + parseInt(liveSearch.css('borderLeftWidth'), 10) + parseInt(liveSearch.css('borderRightWidth'), 10);

		// Re calculates live search's position
		var repositionLiveSearch = function () {
			var tmpOffset	= input.offset();
			var inputDim	= {
				left:		tmpOffset.left, 
				top:		tmpOffset.top, 
				width:		input.outerWidth(), 
				height:		input.outerHeight()
			};

			inputDim.topPos		= inputDim.top + inputDim.height;
			inputDim.totalWidth	= inputDim.width - liveSearchPaddingBorderHoriz;

			liveSearch.css({
				position:	'absolute', 
				left:		inputDim.left + 'px', 
				top:		inputDim.topPos + 'px',
				width:		inputDim.totalWidth + 'px'
			});
		};

		// Shows live-search for this input
		var showLiveSearch = function () {
			// Always reposition the live-search every time it is shown
			// in case user has resized browser-window or zoomed in or whatever
			repositionLiveSearch();

			// We need to bind a resize-event every time live search is shown
			// so it resizes based on the correct input element
			jQuery(window).unbind('resize', repositionLiveSearch);
			jQuery(window).bind('resize', repositionLiveSearch);

			liveSearch.slideDown(config.duration);
		};

		// Hides live-search for this input
		var hideLiveSearch = function () {
			liveSearch.slideUp(config.duration, function () {
				config.onSlideUp();
			});
		};

		input
			// On focus, if the live-search is empty, perform an new search
			// If not, just slide it down. Only do this if there's something in the input
			.focus(function () {
				if (this.value !== '') {
					// Perform a new search if there are no search results
					if (liveSearch.html() == '') {
						this.lastValue = '';
						input.keyup();
					}
					// If there are search results show live search
					else {
						// HACK: In case search field changes width onfocus
						setTimeout(showLiveSearch, 1);
					}
				}
			})
			// Auto update live-search onkeyup
			.keyup(function () {
				// Don't update live-search if it's got the same value as last time
				if (this.value != this.lastValue) {
					input.addClass(config.loadingClass);

					var q = this.value;

					// Stop previous ajax-request
					if (this.timer) {
						clearTimeout(this.timer);
					}

					// Start a new ajax-request in X ms
					this.timer = setTimeout(function () {
						jQuery.get(config.url + q, function (data) {
							input.removeClass(config.loadingClass);

							// Show live-search if results and search-term aren't empty
							if (data.length && q.length) {
								liveSearch.html(data);
								showLiveSearch();
							}
							else {
								hideLiveSearch();
							}
						});
					}, config.typeDelay);

					this.lastValue = this.value;
				}
			});
	});
};

[westonruter]

I don't think the problem is with the JS. The problem is with the search results. For example, you can see that this is an invalid AMP page: https://www.odevbul.org/?livesearch=used&s=test

[westonruter]

What code is responsible for serving the live search results?

[tryazi]

<!-- Start of Search Wrapper -->
<?php
$theme_search_banner_image = get_option( 'theme_search_banner_image' );
if ( ! empty( $theme_search_banner_image ) ) {
?>
<div class="search-area-wrapper search-area-theme-option" style="background-image: url('<?php echo esc_url( $theme_search_banner_image ); ?>')">
	<?php
} else {
	?>
	<div class="search-area-wrapper">
		<?php
}
		?>
		<div class="search-area container">
			<?php
			$theme_search_title = stripslashes( get_option( 'theme_search_title' ) );
			$theme_search_text  = stripslashes( get_option( 'theme_search_text' ) );
			?>
			<h3 class="search-header"><?php echo esc_html( $theme_search_title ); ?></h3>
			<p class="search-tag-line"><?php echo wp_kses_post( $theme_search_text ); ?></p>

			<form id="search-form" class="search-form clearfix" method="get" action="<?php echo esc_url( home_url( '/' ) ); ?>" autocomplete="off">
				<input class="search-term required" type="text" id="s" name="s" placeholder="<?php esc_html_e( 'Type your search terms here', 'framework' ); ?>" title="<?php esc_html_e( '* Please enter a search term!', 'framework' ); ?>"/>
				<input class="search-btn" type="submit" value="<?php esc_html_e( 'Search', 'framework' ); ?>"/>
				<div id="search-error-container"></div>
			</form>
		</div>
	</div>
	<!-- End of Search Wrapper -->

[westonruter]

This is search results form, not the live search results template. What is generating this: https://www.odevbul.org/?livesearch=used&s=test

[tryazi]

    /*-----------------------------------------------------------------------------------*/
    /*	Live Search
     /*-----------------------------------------------------------------------------------*/
    if(jQuery().liveSearch && typeof localized !== 'undefined' ){
        $('#s').liveSearch({
            url: localized.home_url + '?livesearch=used&s='
        });
    }

or

<?php
if ( isset( $_GET['livesearch'] ) ) :

	// if livesearch is used then generate results in list only.
	echo '<ul id="live-search-results" class="clearfix">';

	if ( have_posts() ) :
		while ( have_posts() ) :
			the_post();

			$format = get_post_format();
			if ( false === $format ) {
				$format = 'standard';
			}
			if ( 'faq' === $post->post_type ) {
				$format = 'faq';
			}
			if ( 'topic' === $post->post_type ) {
				$format = 'topic';
			}
			if ( 'reply' === $post->post_type ) {
				$format = 'reply';
			}
			if ( 'forum' === $post->post_type ) {
				$format = 'forum';
			}

			?>
			<li class="search-result <?php echo esc_html( $format ); ?>">
				<a href="<?php the_permalink(); ?>">
					<?php
					inspiry_post_format_icon( $format );
					the_title();
					?>
				</a>
				<?php if ( 'post' === $post->post_type ) : ?>
					<span class="like-count"><i class="fa fa-thumbs-o-up"></i><?php echo esc_html( get_total_likes( $post->ID ) ); ?></span>
				<?php endif; ?>
			</li>
		<?php
		endwhile;
	else :
		?>
		<li class="no-result"><?php esc_html_e( 'No Results Found!', 'framework' ); ?></li>
	<?php
	endif;

	echo '</ul>';   // end of list.

// else generate full page markup.
else:
	get_header();
	?>

I couldn't find another code.

[westonruter]

I think the problem is that you are serving an HTML fragment, where you are lacking the root <html> element.

Try adding this code to your theme's functions.php or a custom plugin:

add_filter( 'amp_is_enabled', function( $enabled ) {
    if ( isset( $_GET['livesearch'] ) ) {
        $enabled = false;
    }
    return $enabled;
} );

[tryazi]

it worked. thank you.

[amedina]

@tryazi Please consider rating the plugin on WordPress.org. Thanks!

[westonruter]

I'm going re-open this because the underlying issue still needs to be resolved. I've updated the issue title and description with what needs to be done.

[tryazi]

You can add an update with the use of accordion to the milestone. I'm using accordion on my site and it doesn't work properly.

Sample url and js file.

https://www.odevbul.org/anyon-ve-katyon-nedir.html

` /* ---------------------------------------------------- /
/ Accordion
/* ---------------------------------------------------- */
$(function() {
$('.accordion dt').click(function(){
$(this).siblings('dt').removeClass('current');
$(this).addClass('current').next('dd').slideDown(500).siblings('dd').slideUp(500);
});
});

/* ---------------------------------------------------- */
/*	Toggle
/* ---------------------------------------------------- */
$(function() {
	$('dl.toggle dt').click(function(){
		if($(this).hasClass('current')){
			$(this).removeClass('current').next('dd').slideUp(500);
		}				
		else{
			$(this).addClass('current').next('dd').slideDown(500);
		}
	});	 
});`

The amp was excluded because it didn't work properly. Click on the content is not seen. There must be hundreds of pages and it's hard to find and edit them.

The plugin should automatically translate it.

If I use amp acordion tags, it does.

<accordion></accordion>text this is a parable.

[westonruter]

@tryazi please open a new issue regarding that. I don't think automatic conversion of accordions would be in scope for the plugin. It would be up to your theme to output a proper <amp-accordion> element.

[westonruter]

Closely related to #2968 (comment), namely:

The v0.js and AMP boilerplate style should really be getting injected by the post-processor anyway (see also #1778).