ampproject / amp-wp

Enable AMP on your WordPress site, the WordPress way.
https://wordpress.org/plugins/amp/
GNU General Public License v2.0
1.78k stars 381 forks source link

Plugin is Breaking Media Library Image "Pre-selection" #4414

Closed davidbhayes closed 4 years ago

davidbhayes commented 4 years ago

Bug Description

On our website, when I click a media image (let's say the Featured Image), I'm used to WordPress "selecting" that image for me in the media modal popup. When I enable the AMP plugin, I no longer see this behavior. When I disable the AMP plugin, I do see the the behavior.

Expected Behaviour

Please note that I am using the WordPress Gutenberg/Block editor here. I've not tested/replicated this in the "Classic" editor. Also, this is what happens when AMP is not running.

  1. I go to edit an (already existing) post on my WordPress site. (It has a featured image set.)
  2. I click the featured image on the post.
  3. When I am transported into the media modal, I see the image which is selected as the featured image highlighted.

Steps to reproduce

Install and activate the AMP plugin. The above steps no longer work for me.

Screenshots

Screen Shot 2020-03-20 at 12 17 20 PM Screen Shot 2020-03-20 at 12 17 13 PM

What's important to note about the above images is that my jars of peanut butter are the "Featured Image". But when I click that image, I do not see it selected in the media-modal.

Additional context

Site Health Massive Dump:

`

wp-core

version: 5.3.2 site_language: en_US user_language: en_US timezone: America/Chicago permalink: /%category%/%postname%/ https_status: false user_registration: 0 default_comment_status: closed multisite: false user_count: 1050 dotorg_communication: true

wp-paths-sizes

wordpress_path: /Users/david/Dropbox/WebDev/ddwp wordpress_size: 3.98 GB (4272919353 bytes) uploads_path: /Users/david/Dropbox/WebDev/ddwp/wp-content/uploads uploads_size: 6.45 KB (6603 bytes) themes_path: /Users/david/Dropbox/WebDev/ddwp/wp-content/themes themes_size: 122.17 MB (128107651 bytes) plugins_path: /Users/david/Dropbox/WebDev/ddwp/wp-content/plugins plugins_size: 94.17 MB (98747754 bytes) database_size: 4.62 GB (4955815936 bytes) total_size: 8.81 GB (9455597297 bytes)

wp-active-theme

name: FreePress UI (freepress-ui) version: 1.0.0 author: FreePress Dev Team author_website: http://underscores.me/ parent_theme: none theme_features: automatic-feed-links, title-tag, post-thumbnails, menus, html5, custom-background, customize-selective-refresh-widgets, custom-logo, editor-color-palette, custom-header, amp, widgets theme_path: /Users/david/Dropbox/WebDev/ddwp/wp-content/themes/freepress-ui

wp-themes-inactive (2)

dailydot: version: 1.1.0, author: Arsenio Santos Twenty Sixteen: version: 2.0, author: the WordPress team

wp-plugins-active (2)

AMP: version: 1.4.4, author: AMP Project Contributors DD Images from Production (BE Media from Production, lightly changes): version: 1.5.0, author: Bill Erickson

wp-plugins-inactive (43)

Ads.txt Manager: version: 1.2.0, author: 10up AdThrive Ads: version: 1.0.37, author: AdThrive Application Passwords: version: 0.1.1, author: George Stephanis Automatic Asset Versioning: version: 0.1.2, author: 10up Autoptimize: version: 2.6.2, author: Frank Goossens (futtta) Cloudflare: version: 3.4.1, author: John Wineman, Furkan Yilmaz, Junade Ali (Cloudflare Team) Co-Authors Plus: version: 3.4.2, author: Mohammad Jangda, Daniel Bachhuber, Automattic Daily Dot + AMP + CafeMedia: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot + Blueconic: version: 1.0.1, author: Daily Dot (davidbhayes) Daily Dot + Complex ads: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot + Google News Keywords: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot + Google Tag Manager: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot + Taboola: version: 1.0.1, author: Daily Dot (davidbhayes) Daily Dot + WPE Geotagging: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot Blocks (Gutenberg): version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot JS Deferer and CSS Remover: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot Photo Credits: version: 1.0.0, author: Daily Dot (davidbhayes) Daily Dot Prebid: version: 1.1.0, author: Daily Dot Disable Comments: version: 1.10.2, author: Samir Shah Distributor: version: 1.2.3, author: 10up Inc. Dynamite: version: 1.1.7, author: Daily Dot Emma For WordPress (Forked by 10up): version: 1.3.3, author: Ah So & 10up EWWW Image Optimizer Cloud: version: 5.2.2, author: Exactly WWW Forget About Shortcode Buttons: version: 2.1.2, author: Designs & Code Glue for Yoast SEO & AMP: version: 0.6, author: Joost de Valk Gravity Forms: version: 2.4.17, author: rocketgenius Header Footer Code Manager: version: 1.1.6, author: 99robots Instant Articles for WP: version: 4.2.0, author: Automattic, Dekode, Facebook Multiple Featured Images: version: 0.5.0, author: Marcus Kober Noindex Pagination Pages: version: 1.2, author: FoundationDigital OneSignal Push Notifications: version: 2.1.1, author: OneSignal OptinMonster API: version: 1.9.6, author: OptinMonster Team PHP Compatibility Checker: version: 1.5.0, author: WP Engine Publish to Apple News: version: 2.0.3, author: Alley Simple Local Avatars: version: 2.1.1, author: Jake Goldman, 10up Taxonomy Meta: version: 1.2, author: Rilwis Term Management Tools: version: 1.1.4, author: scribu User Role Editor: version: 4.53, author: Vladimir Garagulya WP Bitly: version: 2.5.2, author: Temerity Studios WP Engine Automated Migration: version: 3.4, author: WPEngine WP Engine Content Performance: version: 1.5.3+627.25f0b62, author: WP Engine Yoast SEO: News: version: 12.2, author: Team Yoast Yoast SEO Premium: version: 12.5.1, author: Team Yoast

wp-media

image_editor: WP_Image_Editor_GD imagick_module_version: Not available imagemagick_version: Not available gd_version: bundled (2.1.0 compatible) ghostscript_version: not available

wp-server

server_architecture: Darwin 18.7.0 x86_64 httpd_software: nginx/1.15.12 php_version: 7.4.3 64bit php_sapi: fpm-fcgi max_input_variables: 1000 time_limit: 30 memory_limit: 256M max_input_time: 60 upload_max_size: 128M php_post_max_size: 128M curl_version: 7.68.0 OpenSSL/1.1.1d suhosin: false imagick_availability: false

wp-database

extension: mysqli server_version: 5.7.25 client_version: mysqlnd 7.4.3

wp-constants

WP_HOME: undefined WP_SITEURL: undefined WP_CONTENT_DIR: /Users/david/Dropbox/WebDev/ddwp/wp-content WP_PLUGIN_DIR: /Users/david/Dropbox/WebDev/ddwp/wp-content/plugins WP_MAX_MEMORY_LIMIT: 256M WP_DEBUG: false WP_DEBUG_DISPLAY: true WP_DEBUG_LOG: false SCRIPT_DEBUG: false WP_CACHE: false CONCATENATE_SCRIPTS: undefined COMPRESS_SCRIPTS: undefined COMPRESS_CSS: undefined WP_LOCAL_DEV: undefined DB_CHARSET: utf8 DB_COLLATE: undefined

wp-filesystem

wordpress: writable wp-content: writable uploads: writable plugins: writable themes: writable

`


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

westonruter commented 4 years ago

Thanks for the report. I can indeed replicate this issue.

I suspect the problematic code is in select-media-frame.js.

westonruter commented 4 years ago

@davidbhayes Please test this build: https://github.com/ampproject/amp-wp/pull/4453#issuecomment-604839285.

davidbhayes commented 4 years ago

@westonruter thanks for making the ZIP to test :)

It's certainly an improvement. Here's what I'm seeing:

1) Now, in the right of the media popup panel I'm seeing the image correctly showing reliably. 2) In the left (grid of images) part of the panel, I'm (still) not seeing the image selected with a check-mark, how I expect to from years or WP use. I will say that Gutenberg (without AMP) seems to intermittently behave this way, so it could be its issue not yours. What I expect is that in the gird I'll see my image (on an older post) selected and "pulled" to the top-left of the whole grid. What I get with AMP on (and sometimes without) is that IF the image is in the grid, it doesn't pull to the top left, but is highlighted. It seems to happen MORE while testing with this than I've seen on any other WP site, but I can't say for sure.

I recorded a video about it which does clearly demonstrate an "AMP being on causes the issue" condition if it's helpful: https://www.dropbox.com/s/6x7exppmyybx4k1/amp-image-report.mp4?dl=0

pierlon commented 4 years ago

Hi @davidbhayes,

Thanks for the quick reply and such a detailed video demonstrating the problem.

I'll be taking a look at this within the day and will get back to you on it :smile:.

westonruter commented 4 years ago

@davidbhayes Here's a new build for you to test: amp.zip (1.5.2-alpha-20200401T201351Z-686e4454d)

Assuming it works for you, this may be released tomorrow in v1.5.2.

westonruter commented 4 years ago

@davidbhayes Updated build to test: amp.zip (v1.5.2-alpha-20200402T210847Z-614c58b92)

davidbhayes commented 4 years ago

Just tried, seems to be working as expected for me. Seeing the image selected on both the left AND right of the media-selection pane. Testing on 5.4 + v1.5.2-alpha-20200402T210847Z-614c58b92 is 👍