CSS.registerProperty: detect depedency cycles from length units |
||
Issue descriptionFor registered properties, the following should not be valid: --font-size: 10em; font-size: var(--font-size); When resolving the value of font-size, we need to detect that the var() has a cycle, and discard the declaration. The same goes for other font dependent units. https://github.com/w3c/css-houdini-drafts/issues/315
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fcc2b1720620f0004b5f34d4468f708bf76d5799 commit fcc2b1720620f0004b5f34d4468f708bf76d5799 Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Tue Jul 10 09:10:13 2018 Use a single Result object when resolving custom variable references. New result flags are about to be added. This refactor makes that patch less noisy. Bug: 848698 Change-Id: I7e7495ba2265a7ffdc329bab59e772c4ed5e65ff Reviewed-on: https://chromium-review.googlesource.com/1120329 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Anders Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#573656} [modify] https://crrev.com/fcc2b1720620f0004b5f34d4468f708bf76d5799/third_party/blink/renderer/core/css/resolver/css_variable_resolver.cc [modify] https://crrev.com/fcc2b1720620f0004b5f34d4468f708bf76d5799/third_party/blink/renderer/core/css/resolver/css_variable_resolver.h
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58da3151773fb9b5d8607b540c2a01fc05a1e6f8 commit 58da3151773fb9b5d8607b540c2a01fc05a1e6f8 Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Tue Jul 10 09:15:36 2018 Use an Options object to propagate flags when resolving var-references. Two new flags will be added soon. This patch makes that future diff cleaner. Note: The reason the Options object is private is that the two new flags I had in mind can be set in ResolveVariableReferences from the incoming CSSPropertyID and the StyleResolverState. Bug: 848698 Change-Id: I644681d0a199e4b2727d550f51535f3e140be44a Reviewed-on: https://chromium-review.googlesource.com/1120330 Commit-Queue: Anders Ruud <andruud@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#573657} [modify] https://crrev.com/58da3151773fb9b5d8607b540c2a01fc05a1e6f8/third_party/blink/renderer/core/css/resolver/css_variable_resolver.cc [modify] https://crrev.com/58da3151773fb9b5d8607b540c2a01fc05a1e6f8/third_party/blink/renderer/core/css/resolver/css_variable_resolver.h
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c13a893ebf9a176c754209b23955a471d0313afb commit c13a893ebf9a176c754209b23955a471d0313afb Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Tue Jul 10 11:52:23 2018 Detect font units in CSSVariableData. The CSSWG has decided that the following is not permissible, for a registered custom property --foo: --foo: 10em; font-size: var(--foo); This is because --foo depends on font-size via the 'em' unit, and font-size depends on --foo via the var reference. In order to detect this situation, we have to know whether a certain CSSVariableData contains font units or not. This patch adds that capability, however, it is currently unused information. A subsequent patch will implement the actual check. Bug: 848698 Change-Id: I7861a089afc89c4fe7b22ad4f056f92e6248978a Reviewed-on: https://chromium-review.googlesource.com/1120331 Commit-Queue: Anders Ruud <andruud@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#573685} [modify] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/css/css_variable_data.cc [modify] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/css/css_variable_data.h [add] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/css/resolver/css_variable_data_test.cc [modify] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/css/resolver/css_variable_resolver.cc [modify] https://crrev.com/c13a893ebf9a176c754209b23955a471d0313afb/third_party/blink/renderer/core/css/style_environment_variables.cc
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ab1f973716a535b39e6f10843a992e1a1ec26ae commit 4ab1f973716a535b39e6f10843a992e1a1ec26ae Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Tue Jul 10 13:59:32 2018 Disallow font-relative units when resolving var() for font-size. For a registered custom property --foo, the following is no longer allowed: --foo: 10em; font-size: var(--foo); The same applies to ex and ch, as well as rem on the root element. This is because --foo depends on font-size via the 'em' unit, and font-size depends on --foo via the var() reference. This creates a circular dependency that must be resolved somehow. The agreed-upon way to resolve that, has been by treating the whole var() reference as invalid. This patch implements that cycle detection by adding flags to disallow resolution of any var() references that contain font-relative units. These flags are set in ResolveVariableReferences based on the CSSPropertyID/StyleResolverState. When a custom property itself contains var() references, it needs to "inherit" the font-unit-ness from the resolved variables (similar to is_animation_tainted). This is why flags have also been added to Result. Bug: 848698 Change-Id: Ife0570949d996c5fc26dc10b1d7ca15bd27343b1 Reviewed-on: https://chromium-review.googlesource.com/1120334 Commit-Queue: Anders Ruud <andruud@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#573712} [modify] https://crrev.com/4ab1f973716a535b39e6f10843a992e1a1ec26ae/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/registered-property-computation.html [add] https://crrev.com/4ab1f973716a535b39e6f10843a992e1a1ec26ae/third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/unit-cycles.html [modify] https://crrev.com/4ab1f973716a535b39e6f10843a992e1a1ec26ae/third_party/blink/renderer/core/css/resolver/css_variable_resolver.cc [modify] https://crrev.com/4ab1f973716a535b39e6f10843a992e1a1ec26ae/third_party/blink/renderer/core/css/resolver/css_variable_resolver.h
,
Jul 13
|
||
►
Sign in to add a comment |
||
Comment 1 by andruud@chromium.org
, Jul 2