[amp-story] AmpStory runtime replacing vh/vw in css classnames

[nikse]

What's the issue?

Looks like the amp runtime is being a little over aggressive in it's vh/vw replacements as it's replacing the pattern /\d+v[hw]/i in class names effectively breaking the selectors and css

i.e. .cardText__1VW gets turned into .cardText__calc(1 * var(--story-page-vw))

How do we reproduce the issue?

Add a class to an element with the pattern above and render the html in amp

What browsers are affected?

All

Which AMP version is affected?

Seemingly just amp stories, but no clue where else this could be affecting

[twifkak]

cc @ampproject/wg-caching

[kristoferbaxter]

@gmajoulet This appears to be an issue with Stories, would you mind taking a look?

[gmajoulet]

Thanks for the bug report. We'll look into this this week. By order of preference, we should try to:

1. Improve the regexp and not match what looks like a CSS selector (would excluding "starts with ." or "starts with #" be enough?)
2. Add a validation rule and make vw/vh invalid in Stories CSS selector, which should be just like the i-amphtml- disallowed prefix. This might make some stories invalid though.

cc @ampproject/wg-stories

[newmuis]

2 might be pretty hard for some folks to meet if they're using CSS minifiers that obfuscate class names

[Gregable]

I don't think #2 is a good idea. It's a pretty easy to match regex by accident and would be unfortunate for publishers to hit. Hence, I'm removing WG: Caching from the list here.

Could the stores runtime actually use something more sophisticated than a CSS regex and match only parts of the CSS which are property values? I think regex on the entire CSS string is almost certainly going to be error-prone.

[gmajoulet]

Sounds good, let's go with option 1 "Improve the regexp and not match what looks like a CSS selector". We will prioritize this higher and get to it in the next two weeks.

[gmajoulet]

I gave it a try and could only get it to work using lookahead/lookbehind regex features, that aren't working on all browsers.

I looked at many documents, uncovered a few edge cases, and did come up with what I think is an okay test string:

.foo {
  width:3vh;
  height: 5vh
  ;
}

.foo3vh {
  --hello3vh: 3vh;
  width: 3vh
}

.foo3vh {
  --hello3vh: 3vh;
  width: 3vh}

foo {transform: translate3d(-100vh, -100vh, 0);} .cardText__1vh {}

.cardText__1vh {}

.cardText1vh {}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.