Skip to content

Show error when attempting to validate an AMP-unavailable URL instead of forcing AMP to be available #5296

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

Merged
merged 8 commits into from
Apr 8, 2021

Conversation

westonruter
Copy link
Member

Summary

Fixes #5295

This error is shown now when attempting to validate a URL that is no longer available as AMP:

image

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added this to the v2.1 milestone Aug 29, 2020
@google-cla google-cla bot added the cla: yes Signed the Google CLA label Aug 29, 2020
@westonruter westonruter added the WS:Core Work stream for Plugin core label Sep 3, 2020
@CLAassistant
Copy link

CLAassistant commented Oct 23, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #5296 (87ffec7) into develop (e21a628) will increase coverage by 0.00%.
The diff coverage is 84.21%.

❗ Current head 87ffec7 differs from pull request most recent head 24583ad. Consider uploading reports for the commit 24583ad to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #5296   +/-   ##
==========================================
  Coverage      75.39%   75.39%           
- Complexity      5700     5706    +6     
==========================================
  Files            218      218           
  Lines          17255    17270   +15     
==========================================
+ Hits           13009    13021   +12     
- Misses          4246     4249    +3     
Flag Coverage Δ Complexity Δ
javascript 80.05% <ø> (ø) 0.00 <ø> (ø)
php 75.19% <84.21%> (+<0.01%) 0.00 <4.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
includes/amp-helper-functions.php 84.48% <ø> (-0.06%) 0.00 <0.00> (ø)
includes/class-amp-theme-support.php 86.31% <66.66%> (-0.24%) 245.00 <0.00> (+3.00) ⬇️
...cludes/validation/class-amp-validation-manager.php 80.82% <100.00%> (+0.18%) 283.00 <4.00> (+3.00)

@pierlon pierlon self-assigned this Apr 1, 2021
@pierlon pierlon marked this pull request as ready for review April 1, 2021 10:00
@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2021

Plugin builds for 24583ad are ready 🛎️!

@pierlon
Copy link
Contributor

pierlon commented Apr 1, 2021

@westonruter Asking for your review as I can't assign you as one 🙂.

…ating-amp-unavailable-urls

* 'develop' of github.com:ampproject/amp-wp: (24 commits)
  Update DetermineHeroImagesTest to use amp-img instead of img
  Prevent DetermineHeroImages transformer from targeting any img element
  Avoid identifying noscript fallback img as hero image candidates
  Bump eslint-plugin-jest from 24.3.2 to 24.3.4
  Bump postcss from 8.2.8 to 8.2.9
  Bump svgo from 2.2.2 to 2.3.0
  Bump @babel/core from 7.13.10 to 7.13.14
  Bump eslint from 7.22.0 to 7.23.0
  Bump classnames from 2.2.6 to 2.3.1
  Add covers phpdoc tags
  Fix tests and optimize attribute removal
  Restore object-fit:contain for a reduced-specificity selector
  Omit problematic styling attributes from noscript fallback elements
  Fix inheritance of replaced-content properties on noscript fallbacks
  Fix double arrow alignment
  Remove obsolete core sanitizer CSS code from 2019, 2017, and 2014
  Inherit replaced-content properties for replaced content shadow elements and noscript fallbacks
  Remove obsolete object-fit:contain for unknown-sized image since redundant with object-fit=contain attr
  Bump postcss-import from 14.0.0 to 14.0.1
  Restrict object-fit to images which have the .amp-wp-unknown-size class
  ...
@westonruter westonruter requested a review from pierlon April 8, 2021 04:04
Copy link
Contributor

@pierlon pierlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@westonruter westonruter merged commit 81b9df7 into develop Apr 8, 2021
@westonruter westonruter deleted the fix/validating-amp-unavailable-urls branch April 8, 2021 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-singular URLs remain able to be validated after switching to legacy reader mode
3 participants