New issue
Advanced search Search tips

Issue 851490 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 641877



Sign in to add a comment

Registered properties need proper CSSParserContext for URL resolution

Project Member Reported by andruud@chromium.org, Jun 11 2018

Issue description

For registered properties, relative URLs should resolve against the base URL of the stylesheet in which they are declared.

This does currently not work because we do not keep the original CSSParserContext around for use at moment the value is actually parsed.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/146d766c73164c7cfe3fb28509512e92698bfa53

commit 146d766c73164c7cfe3fb28509512e92698bfa53
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Fri Jun 22 16:54:57 2018

Correctly resolve relative URLs for registered custom properties.

Relative URLs in registered custom properties must resolve against the
base URL of the originating stylesheet. For instance, consider a style-
sheet at /style/mystyle.css:

  --foo: url("myimage.jpg");

And a document at /index.html:

  <style>
    background-image: var(--foo);
  </style>

If the property --foo is registered with syntax <url>, then the background-
image should be /style/myimage.jpg.

This is contrary to non-registered properties, in which case the
background-image would be /myimage.jpg.

To implement this, this patch scans for URL tokens and 'url('-function
tokens after (var-)resolving the registered custom property
(see CSSVariableResolver::ResolveCustomProperty). The token containing
the relative URL is then rewritten (in-place) to contain an absolute URL
instead.

To avoid doing unnecessary work, we only scan the token stream if

CSSVariableData::needs_variable_resolution_ is set. This is set either
if needs_url_resolution_ is set (because var-references could produce
relative URLs), or if the token stream contains URL/function-tokens.
When a resolved CSSVariableData is created (::CreateResolved), it is
assumed that the incoming token stream contains no URLs that need to be
resolved.

R=futhark@chromium.org, timloh@chromium.org

Bug:  851490 
Change-Id: I25b1e839fc92eb538f30670fe91fc92a1ad9d5ea
Reviewed-on: https://chromium-review.googlesource.com/1109975
Commit-Queue: Anders Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569663}
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/support/alt/alt.css
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/support/alt/alt.js
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/support/main/main.css
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/support/main/main.js
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/support/main/main.utf16be.css
[add] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/url-resolution.tentative.html
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/animation/css_interpolation_type.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/css_syntax_descriptor.h
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/css_variable_data.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/css_variable_data.h
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/cssom/css_unparsed_value.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/cssom/style_value_factory.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/parser/css_parser_impl.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/parser/css_property_parser.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/parser/css_property_parser_helpers.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/parser/css_variable_parser.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/parser/css_variable_parser.h
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/property_registration.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/resolver/css_variable_resolver.cc
[modify] https://crrev.com/146d766c73164c7cfe3fb28509512e92698bfa53/third_party/blink/renderer/core/css/resolver/css_variable_resolver.h

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31f4d3d2e73367ffde3dfa4184f446123ae35874

commit 31f4d3d2e73367ffde3dfa4184f446123ae35874
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Tue Jul 03 08:35:01 2018

[css-properties-values-api]: Drop .tentative from URL resolution test.

R=eae@chromium.org

Bug:  851490 
Change-Id: I0090986f81763d3c84f1245d5340286f9a3a1dff
Reviewed-on: https://chromium-review.googlesource.com/1121463
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572145}
[rename] https://crrev.com/31f4d3d2e73367ffde3dfa4184f446123ae35874/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/url-resolution.html

Sign in to add a comment