New issue
Advanced search Search tips

Issue 850072 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 propeties are untyped upon enumeration

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

Issue description

Do someElement.computedStyleMap().forEach(p => console.log(p[0])) on an element with a value set for a registered (inherited) custom property. The value appears, but as a CSSUnparsedValue rather than the type from the registration.
 
Status: Started (was: Assigned)
Components: Blink>CSS
Labels: -Pri-3 Pri-2
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2018

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

commit a634522b49117a9816f83066d6adef863d27f985
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Fri Jun 08 10:44:33 2018

CSS Typed OM: Proper types for custom props during iteration.

Currently, you get CSSUnparsedValues when iterating entries on the
.computedStyleMap(), even for properties which are registered with a
type. However, if you do .computedStyleMap().get(...) with a registered
property, you _do_ get the correct type.

This is because ComputedStyleCSSValueMapping::Get properly looks up the
typed values on the ComputedStyle, but ::GetVariables does not; instead it
directly returns the CSSVariableData for everything, regardless of
registrations.

This patch changes the behavior of iteration to use the same code path
as .get(). This ensures that we either create a CSSCustomProperty-
Declaration on the fly for unregistered properties, or we re-use the
existing (and typed) CSSValue for registered properties.

Note: I initially wanted to do a nested iterator thing here, but dropped
      it because we would anyway need a temp HashSet to avoid potential
      duplicates from the StyleInheritedVariables-root.

Note: One caller of GetVariables() just wants the size. It's possibly a
      little overkill to create entire HashMap just to count something,
      but I don't want to resolve that here in the same patch.

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

Bug:  850072 
Change-Id: I64bfb3cc22a377cf956420a9e64d47ca3daac1e2
Reviewed-on: https://chromium-review.googlesource.com/1090848
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565605}
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/WebKit/LayoutTests/external/wpt/css/cssom/computed-style-set-property.html
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties-expected.txt
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/css/computed_style_css_value_mapping.cc
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/css/computed_style_css_value_mapping.h
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/css/css_computed_style_declaration.cc
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/css/css_computed_style_declaration.h
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/css/cssom/computed_style_property_map.cc
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/inspector/inspector_css_agent.cc
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/style/style_inherited_variables.cc
[modify] https://crrev.com/a634522b49117a9816f83066d6adef863d27f985/third_party/blink/renderer/core/style/style_inherited_variables.h

Status: Fixed (was: Started)

Sign in to add a comment