Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude images with property="og:image" from serving the WebP version. #3073

Closed
4 tasks
kierantaylorio opened this issue Sep 7, 2020 · 7 comments · Fixed by #3607
Closed
4 tasks

Exclude images with property="og:image" from serving the WebP version. #3073

kierantaylorio opened this issue Sep 7, 2020 · 7 comments · Fixed by #3607
Assignees
Labels
effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: webp cache priority: low Issues that can wait type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@kierantaylorio
Copy link
Contributor

Before submitting an issue please check that you’ve completed the following steps:
👍 - Made sure you’re on the latest version
👍 - Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug

When WP Rockets WebP caching is active we will rewrite the image URLs including those in meta tags. This means that a WebP image is served when sharing via WhatsApp. This is not supported and therefore the image will not be loaded.

To Reproduce
Steps to reproduce the behaviour:

  1. Activate Imagify (with 'Display images in WebP format on the site' disabled)
  2. Enable 'Enable WebP caching' in WPR
  3. Use a plugin like Easy Social Share to add buttons to share via WhatsApp
  4. Check the source for the meta tag and you should see the WebP version of the image
  5. Share via WhatsApp

Expected behaviour

The image should appear when sharing via WhatsApp, however, this will not happen as WebP is not supported.

Additional context

Related ticket: https://secure.helpscout.net/conversation/1274339485/192450

This can be resolved by enabling Display images in webp format on the site > Use tags (preferred) in Imagifys settings.

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@arunbasillal arunbasillal added module: media priority: low Issues that can wait type: enhancement Improvements that slightly enhance existing functionality and are fast to implement labels Sep 8, 2020
@arunbasillal
Copy link
Contributor

As per: https://developers.facebook.com/docs/sharing/webmasters/#images

MIME type of the image. One of image/jpeg, image/gif or image/png

It looks like excluding image/webp is indeed required. Thanks for the detailed report @kierantaylorio

@Tabrisrp
Copy link
Contributor

Reproduce the issue ✅

Identify the root cause ✅

In our WebP subscriber class, the method get_attribute_names() has a default value containing the attribute content to search for image files.

This means that images inside the content attribute will be rewritten to WebP if the corresponding webp file exists.

But the WebP format is not accepted as a mime type apparently, so we should not try to replace those.

Scope a solution ✅

Remove the content value from the attribute list in Webp_Subscriber::get_attributes_names()

Estimate the effort ✅

Effort [XS]

@Tabrisrp Tabrisrp added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Dec 15, 2020
@Tabrisrp Tabrisrp added the good first issue Indicates a good issue for first-time contributors label Feb 1, 2021
@Tabrisrp Tabrisrp added this to the 3.8.7 milestone Feb 10, 2021
@jorgemartine00 jorgemartine00 self-assigned this Feb 18, 2021
@Mai-Saad
Copy link

Mai-Saad commented Mar 5, 2021

On trunk:

Am I missing something to see the image problem?

the image will not be loaded.

@Tabrisrp
Copy link
Contributor

Tabrisrp commented Mar 6, 2021

From https://stackoverflow.com/a/64326921, posted on october 15, 2020.

Twitter and Facebook now supports WebP in og:meta.

This change might not be required anymore.

@Mai-Saad If you confirm things are working normally now, we can close the issue.

@piotrbak
Copy link
Contributor

piotrbak commented Mar 9, 2021

@Tabrisrp @Mai-Saad From what I can see:

  1. Facebook Sharing Debugger is automatically rewriting images from .png.webp to .png.
    https://developers.facebook.com/tools/debug/

Also, in their docs, the image/type still doesn't have webp format.
https://developers.facebook.com/docs/sharing/webmasters/#images

  1. Twitter, doesn't seem to work with webp in og:image property. The png seems to be just fine
    https://cards-dev.twitter.com/validator

URL to use, you can compare Twitter with the not cached version:
https://imagify.rocketlabsqa.ovh/hello-world/

@Tabrisrp Tabrisrp mentioned this issue Mar 10, 2021
iCaspar added a commit that referenced this issue Mar 26, 2021
* Add Unit test Warmup/APIClient

* Add unit tests for Frontend APIClient

* Update comments with @uses for AbstractAPI coverage

* Fixes #3083 Update delay JS RegEx to ignore space inside script tags (PR #3587)

* Updating AWX webhook url

* Fixes #3576 Guard rocket_defer_inline_exclusions filter return when used by 3rd parties (PR #3582)

* Fixes #2970 Add new post-type exclusion "cms_block" from CPCSS generation (PR #3550)

* Closes #3423 Add preconnect tag for CDN URLs (#3463)

* Add add_preconnect_cdn() method

* Add integration test draft and notes.

* Implement preconnect for non-crossorigin cdn-urls

* Add fixture, adjust integration test

* Revise forcorrect cdn urls, use crossorigin second

* Run phpcbf

* Update CDN preconnect testcases

* Use agnostic scheme when not provided

* Rerun phpcs

* Clean dns-prefetch out of testcases :)

* Add testcase/solution for `test/tests`

* Closes #3539 Combine @import rules into the minified CSS files (PR #3603)

* update plugin version

* Fix #3625 Contact form 7 stop working if dependency scripts are deferred after latest update (PR #3629)

* exclude files from being deferred
* add hooks script to the exclude list

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in ru_RU

translation completed for the source file '/languages/rocket.pot'
on the 'ru_RU' language.

* update pot file with changed strings

* Translate /languages/rocket.pot in de_DE

translation completed for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in fr_FR

translation completed for the source file '/languages/rocket.pot'
on the 'fr_FR' language.

* update translations

* Translate /languages/rocket.pot in tr_TR

translation completed for the source file '/languages/rocket.pot'
on the 'tr_TR' language.

* Exclude googleoptimize.com from minify/combine JS (PR #3609)

* Exclude www.idxhome.com from combine/defer JS (#3619)

* Exclude wcpv_registration_local from combine JS (PR #3634)

* Add additional inline JS exclusions from combine JS (PR #3638)

* Closes #2883 UI improvement for Never Cache URL placeholder (PR #3631)

* Bump elliptic from 6.5.3 to 6.5.4 (PR #3650)

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.5.3...v6.5.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fixes #3528 Add AMP query string to cached query strings when activating the plugin (PR #3613)

* Tiered Pricing Table for WooCommerce nonce action exclusion (PR #3652)

* Closes #2812 Add new filter to rewrite srcset to the CDN (PR #3648)

* Fixes #2041 CDN: prevent incorrect URL on srcset with duplicated relative URLS (PR #3615)

* Fixes #3114 Apply font-display:swap to CPCSS (PR #3633)

* Fixes #3073 Remove content from attributes list for WebP conversion (PR #3607)

* Fixes #3394 PHP notice in Update_Subscriber::disable_auto_updates() (PR #3632)

* update version to 3.8.7

* v3.8.6 (#3623)

* Update wording in first activation notice on the dashboard (PR #3583)

Add automatically to match the description on the website. Also without it it sounds like WP Rocket would not know how to deal with the remaining 20%.

* Fixes #3590 Imagify icon not shown in Chrome (PR #3591)

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Fixes #3498 Turn off "Delay JS" option when Safe Mode is activated (PR #3574)

* Fixes #3454 Delay JS is not working if scrolling the page with the mouse wheel (PR #3573)

* Closes #2839 Remove Age Verify plugin compatibility files (PR #3585)

* Closes #1744 Show warning when PageSpeed Ninja is active (PR #3580)

* Fixes #3238 Remove empty values in $purge_urls array (PR #3579)

* Fixes #2777 Update text formatting in Rocket Analytics modal (#3598)

* Closes #3113 Stop removing empty lines in the .htaccess (PR #3599)

* Fixes #3605 Replace deprecated jQuery methods (PR #3606)

* Translate /languages/rocket.pot in ru_RU

translation completed for the source file '/languages/rocket.pot'
on the 'ru_RU' language.

* Closes #3564 Update minified filename structure to use query string instead of version in filename (PR #3595)

* Fixes #3083 Update delay JS RegEx to ignore space inside script tags (PR #3587)

* Updating AWX webhook url

* Fixes #3576 Guard rocket_defer_inline_exclusions filter return when used by 3rd parties (PR #3582)

* Fixes #2970 Add new post-type exclusion "cms_block" from CPCSS generation (PR #3550)

* Closes #3423 Add preconnect tag for CDN URLs (#3463)

* Add add_preconnect_cdn() method

* Add integration test draft and notes.

* Implement preconnect for non-crossorigin cdn-urls

* Add fixture, adjust integration test

* Revise forcorrect cdn urls, use crossorigin second

* Run phpcbf

* Update CDN preconnect testcases

* Use agnostic scheme when not provided

* Rerun phpcs

* Clean dns-prefetch out of testcases :)

* Add testcase/solution for `test/tests`

* Closes #3539 Combine @import rules into the minified CSS files (PR #3603)

* update plugin version

* update pot file with changed strings

* Translate /languages/rocket.pot in de_DE

translation completed for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in fr_FR

translation completed for the source file '/languages/rocket.pot'
on the 'fr_FR' language.

* Fix #3625 Contact form 7 stop working if dependency scripts are deferred after latest update (PR #3629)

* exclude files from being deferred
* add hooks script to the exclude list

* update translations

* Translate /languages/rocket.pot in tr_TR

translation completed for the source file '/languages/rocket.pot'
on the 'tr_TR' language.

Co-authored-by: Presskopp <cherrmann@gmx.de>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Buttigieg <jonathan@wp-media.me>
Co-authored-by: Jorge <jorgemartinezmenendez00@gmail.com>
Co-authored-by: Natalia Drause <60236665+NataliaDrause@users.noreply.github.com>
Co-authored-by: Caspar Green <cg@caspar.green>
Co-authored-by: Ahmed Saed <eng.ahmeds3ed@gmail.com>
Co-authored-by: Vasilis Manthos <vmanthos@users.noreply.github.com>
Co-authored-by: Albert Cintas <69654544+AlbertCintas@users.noreply.github.com>
Co-authored-by: Sandy Figueroa <sandyfigueroa@users.noreply.github.com>
Co-authored-by: Soponar Cristina <crystinutzaa@gmail.com>

* Fixes #3658 Force the PSR Container dependency to v1.0.0 (PR #3660)

* Add new inline JS exclusion from combine JS (PR #3683)

* Ignore Pinterest ads "pp" query string when serving the cache (PR #3682)

* Fix double space in renewal banner copy (PR #3674)

* Exclude reload_attached_coupons from combine JS (PR #3671)

* Exclude arf_footer_cl_logic_call from combine JS (PR #3665)

* Fix rest API tests fixtures (PR #3688)

* add details attribute to error data
* remove details attribute from error data and use assertArraySubset to make sure that needed attributes are there
* adjust the GH workflow to use WP 5.7
* adjust the GH workflow to use latest WP version always for non legacy

* Exclude nonce actions for Discount Rules and Dynamic Pricing for WooCommerce (PR #3679)

* update fixtures after typo fix

* Closes #3230 Add PHP 8 to our CI matrix (PR #3656)

* add PHP 8 to our CI matrix
* update composer configuration
* update phpstan-wp dependency version
* update phpunit package version
* update workflows
* only bailout if the provided structure is not empty
* update return typehint for setUp() and setUpBeforeClass()
* use Mockery for mocks instead of phpunit mocks
* remove separate process

* update version to 3.9-alpha1

* v3.8.6 (#3623)

* Update wording in first activation notice on the dashboard (PR #3583)

Add automatically to match the description on the website. Also without it it sounds like WP Rocket would not know how to deal with the remaining 20%.

* Fixes #3590 Imagify icon not shown in Chrome (PR #3591)

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in de_DE

translation completed updated for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Fixes #3498 Turn off "Delay JS" option when Safe Mode is activated (PR #3574)

* Fixes #3454 Delay JS is not working if scrolling the page with the mouse wheel (PR #3573)

* Closes #2839 Remove Age Verify plugin compatibility files (PR #3585)

* Closes #1744 Show warning when PageSpeed Ninja is active (PR #3580)

* Fixes #3238 Remove empty values in $purge_urls array (PR #3579)

* Fixes #2777 Update text formatting in Rocket Analytics modal (#3598)

* Closes #3113 Stop removing empty lines in the .htaccess (PR #3599)

* Fixes #3605 Replace deprecated jQuery methods (PR #3606)

* Translate /languages/rocket.pot in ru_RU

translation completed for the source file '/languages/rocket.pot'
on the 'ru_RU' language.

* Closes #3564 Update minified filename structure to use query string instead of version in filename (PR #3595)

* Fixes #3083 Update delay JS RegEx to ignore space inside script tags (PR #3587)

* Updating AWX webhook url

* Fixes #3576 Guard rocket_defer_inline_exclusions filter return when used by 3rd parties (PR #3582)

* Fixes #2970 Add new post-type exclusion "cms_block" from CPCSS generation (PR #3550)

* Closes #3423 Add preconnect tag for CDN URLs (#3463)

* Add add_preconnect_cdn() method

* Add integration test draft and notes.

* Implement preconnect for non-crossorigin cdn-urls

* Add fixture, adjust integration test

* Revise forcorrect cdn urls, use crossorigin second

* Run phpcbf

* Update CDN preconnect testcases

* Use agnostic scheme when not provided

* Rerun phpcs

* Clean dns-prefetch out of testcases :)

* Add testcase/solution for `test/tests`

* Closes #3539 Combine @import rules into the minified CSS files (PR #3603)

* update plugin version

* update pot file with changed strings

* Translate /languages/rocket.pot in de_DE

translation completed for the source file '/languages/rocket.pot'
on the 'de_DE' language.

* Translate /languages/rocket.pot in fr_FR

translation completed for the source file '/languages/rocket.pot'
on the 'fr_FR' language.

* Fix #3625 Contact form 7 stop working if dependency scripts are deferred after latest update (PR #3629)

* exclude files from being deferred
* add hooks script to the exclude list

* update translations

* Translate /languages/rocket.pot in tr_TR

translation completed for the source file '/languages/rocket.pot'
on the 'tr_TR' language.

Co-authored-by: Presskopp <cherrmann@gmx.de>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Buttigieg <jonathan@wp-media.me>
Co-authored-by: Jorge <jorgemartinezmenendez00@gmail.com>
Co-authored-by: Natalia Drause <60236665+NataliaDrause@users.noreply.github.com>
Co-authored-by: Caspar Green <cg@caspar.green>
Co-authored-by: Ahmed Saed <eng.ahmeds3ed@gmail.com>
Co-authored-by: Vasilis Manthos <vmanthos@users.noreply.github.com>
Co-authored-by: Albert Cintas <69654544+AlbertCintas@users.noreply.github.com>
Co-authored-by: Sandy Figueroa <sandyfigueroa@users.noreply.github.com>
Co-authored-by: Soponar Cristina <crystinutzaa@gmail.com>

* Add integration test for Warmup\APIClient

* Adjust Warmup\APIClient fixture

* Adjust Warmup\APIClient unit test w/new fixture

* Add integration test Frontend\APIClient

* Modify Frontend\APIClient fixture for new test.

* Fix warmup APIClient fixture WPError case

* Add remaining testcases for Frontend APIClient

* Use actual returmed error codes in APIClients

* Refactor check_response()

* Refactor Unit tests for new Fixture

* Clean up fixture

* Refactor Warmup API Unit test for check_response()

* Restore odd removals

* Don’t alias Brain\Monkey\Functions

Co-authored-by: Vasilis Manthos <vmanthos@users.noreply.github.com>
Co-authored-by: Albert Cintas <69654544+AlbertCintas@users.noreply.github.com>
Co-authored-by: Ahmed Saed <eng.ahmeds3ed@gmail.com>
Co-authored-by: Sandy Figueroa <sandyfigueroa@users.noreply.github.com>
Co-authored-by: Rémy Perona <remperona@gmail.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: Adame Dahmani <hello@adame.io>
Co-authored-by: Jorge <jorgemartinezmenendez00@gmail.com>
Co-authored-by: Scott Hartley <45110039+ScottTravisHartley@users.noreply.github.com>
Co-authored-by: Natalia Drause <60236665+NataliaDrause@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rémy Perona <remy@wp-rocket.me>
Co-authored-by: Presskopp <cherrmann@gmx.de>
Co-authored-by: Jonathan Buttigieg <jonathan@wp-media.me>
Co-authored-by: Soponar Cristina <crystinutzaa@gmail.com>
Co-authored-by: Cristina Soponar <45258937+crystinutzaa@users.noreply.github.com>
@sriharshayarabarla
Copy link

Having the same issue. All my images are converted to webp by Smush plugin and I don't see any featured image while sharing in whatsapp.... Facebook and Twitter just works fine with webp images. If I bypass wp-rocket by appending ?nowprocket the featured image shows in whatsapp.

@piotrbak
Copy link
Contributor

piotrbak commented Dec 3, 2021

Hello @sriharshayarabarla thank you for your comment. If you disable our webp addon the problem goes away?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: webp cache priority: low Issues that can wait type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants