Remove space from 'data: url()' in stylesheets #1164
Conversation
amedina
force-pushed
the
amedina/fix-1089
branch
2 times, most recently
from
8cc7e34
to
bbb254f
Compare
tests/test-amp-style-sanitizer.php
Outdated
| * Test handling of stylesheets with spaces in the background-image URLs. | ||
| * | ||
| * @dataProvider get_data_urls | ||
| * @covers AMP_Style_Sanitizer::normalize_urls() |
There was a problem hiding this comment.
I think this is covering AMP_Style_Sanitizer::remove_spaces_from_data_urls() now, right?
tests/test-amp-style-sanitizer.php
Outdated
| * @param string $source Source URL string. | ||
| * @param string|null $expected Expected normalized URL string. | ||
| */ | ||
| public function test_normalize_urls( $source, $expected ) { |
There was a problem hiding this comment.
Likewise, test_ remove_spaces_from_data_urls
tests/test-amp-style-sanitizer.php
Outdated
| 'url_with_spaces' => array( | ||
| 'html { background-image:url(url with spaces.png); }', | ||
| 'html{background-image:url("urlwithspaces.png")}', | ||
| ), |
There was a problem hiding this comment.
Should include data: URL as discussed. Include spaces as reported in support forums.
| if ( empty( $base_url ) ) { | ||
| return; | ||
| } | ||
| private function normalize_urls( $urls, $stylesheet_url = '' ) { |
There was a problem hiding this comment.
I think most of the changes to normalize_urls should be reverted back to real_path_urls because as we found out, the PHP CSS parser fails to parse URLs with spaces in them anyway, so this will never be fed any such URLs. The added comments should be retained, of course.
There was a problem hiding this comment.
Reverted all, except some new code comments.
| $options['stylesheet_url'] | ||
| ); | ||
| } | ||
| $this->normalize_urls( |
There was a problem hiding this comment.
Per below, I think this change can be reverted.
amedina
changed the title
Fixes #1089