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

PHP CSS Parser fails to parse turn CSS unit #4604

Closed
westonruter opened this issue Apr 19, 2020 · 4 comments · Fixed by #5392
Closed

PHP CSS Parser fails to parse turn CSS unit #4604

westonruter opened this issue Apr 19, 2020 · 4 comments · Fixed by #5392
Assignees
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. CSS P1 Medium priority WS:Core Work stream for Plugin core
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Apr 19, 2020

Bug Description

The PHP CSS Parser is failing to parse the CSS units vmin and vmax, inserting spaces so they become vm in and vm ax.

Similarly, the turn unit is also corrupted but in a different way: 1turn gets serialized out as 1 turn which is a syntax error.

Originally reported in the support forum.

Expected Behaviour

All CSS units should be preserved.

Steps to reproduce

Add this HTML to a Custom HTML block:

<div id="box">
I should be 20vmin×20vmax in size (and be spinning)!
</div>
<style>
#box {
  width: 20vmin;
  height: 20vmax;
  border: solid 1px red;
  background-color: gray;
  animation: spin 12s linear infinite;
}
@keyframes spin {
  to {
    transform: rotate(1turn);
  }
}
</style>

Compare the AMP with the non-AMP version. The non-AMP version appears as a box and spins:

image

Whereas the AMP does not (and no animation appears):

image

Update: In develop it the element now appears as a box. The vmax and vmin units were fixed with #4610.

The PHP CSS Parser incorrectly is injecting spaces into the units:

@keyframes spin{
  to {
    transform: rotate(1 turn)
  }
}

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 westonruter added Bug Something isn't working CSS labels Apr 19, 2020
@westonruter westonruter changed the title PHP CSS Parser fails to parse vmin and vmax CSS units PHP CSS Parser fails to parse vmin, vmax, and turn CSS units Apr 20, 2020
@westonruter westonruter added the P1 Medium priority label Apr 20, 2020
@pierlon pierlon assigned pierlon and unassigned pierlon Apr 22, 2020
@raxbg
Copy link

raxbg commented Apr 30, 2020

Hey @westonruter

The issue is already resolved in master - MyIntervals/PHP-CSS-Parser@c4509f5

In fact many issues have been resolved since the last release and master can be considered stable.

Hope this helps.

@westonruter
Copy link
Member Author

@raxbg Thanks for the note.

For reference, in 1.5 branch it is broken:

image

We did actually update to master in #4610 and you can see this is now working for the vmax and vmin units in the develop branch:

image

However the turn unit is still not working. The example is not spinning. The @keyframes is still coming through as:

@keyframes spin{to{transform:rotate(1 turn)}}

It should rather be 1turn.

@westonruter westonruter changed the title PHP CSS Parser fails to parse vmin, vmax, and turn CSS units PHP CSS Parser fails to parse turn CSS unit Apr 30, 2020
@pierlon
Copy link
Contributor

pierlon commented May 6, 2020

Filed MyIntervals/PHP-CSS-Parser#193 to fix this.

@kmyram kmyram added this to the v1.6 milestone May 27, 2020
@westonruter westonruter removed this from the v1.6 milestone Jun 16, 2020
@westonruter westonruter modified the milestone: v2.0 Jul 21, 2020
@kmyram kmyram added the WS:Core Work stream for Plugin core label Aug 5, 2020
@westonruter westonruter added this to the v2.0.2 milestone Sep 17, 2020
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Sep 17, 2020
@westonruter westonruter assigned westonruter and unassigned pierlon Sep 18, 2020
@westonruter
Copy link
Member Author

QA Passed

The turn unit is preserved:

image

Also in a Web Story context:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. CSS P1 Medium priority WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants