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

Patch the PHP-CSS-Parser library to correctly validate the turn unit #5392

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Sep 17, 2020

Summary

Applies the changes from MyIntervals/PHP-CSS-Parser#193 as a patch to be made to the PHP-CSS-Parser library.

Fixes #4604

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).

@google-cla google-cla bot added the cla: yes Signed the Google CLA label Sep 17, 2020
@pierlon pierlon added the WS:Core Work stream for Plugin core label Sep 17, 2020
@pierlon pierlon added this to the v2.0.2 milestone Sep 17, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2020

Plugin builds for 55bdab0 are ready 🛎️!

composer.lock Outdated
"packages": [
{
"name": "ampproject/common",
"version": "dev-develop",
"version": "dev-fix/4604-parse-turn-unit",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You seem to have modified the Composer lock file so that it points to this branch for its libraries. I think that that will break as soon as this is merged, as the composer install will not be able to find the branch that is being referenced in the lock file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be caused by the recent change I did to avoid locking the libraries to a branch so we can cross the v2 branch barrier...

What exactly did you run? Only a composer update in the root folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exactly did you run? Only a composer update in the root folder?

Yep.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copied from Slack:

Ok, I got a usable lock file now. However, I did it by running composer update from dev-develop after having deleted the vendor folder. Then I changed into the fix/4604-parse-turn-unit branch and re-ran composer update without deleting the vendor folder.

@westonruter
Copy link
Member

Changes check out.

I tried creating a Web Story with a gradient:

image

Before After
image image

Also tried adding the test HTML on #4604, here after waiting a couple seconds:

Before After
image image

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Sep 17, 2020
@google-cla
Copy link

google-cla bot commented Sep 17, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no Has not signed the Google CLA and removed cla: yes Signed the Google CLA labels Sep 17, 2020
@schlessera
Copy link
Collaborator

@googlebot I consent.

@google-cla google-cla bot added cla: yes Signed the Google CLA and removed cla: no Has not signed the Google CLA labels Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. 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.

PHP CSS Parser fails to parse turn CSS unit
3 participants