New issue
Advanced search Search tips

Issue 803739 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 545318



Sign in to add a comment

[css-typed-om] Not all CSSMathValues can be serialized.

Project Member Reported by shend@chromium.org, Jan 19 2018

Issue description

Currently, there are some CSSMathValues that can't be serialized because we have not implemented the calc lv4 spec. For example, CSSMathMax's serialization requires supporting the "max" function in <calc> values, which we currently don't have.

There are three temporary solutions:
A) Throw exception when serialization will result in a string that will not parse.
B) Return ""
C) Follow the spec anyway, with the caveat that the result might not roundtrip.

We'll implement A) first because it's easy and explicit. If this turns out to be burdensome to developers, we can implement C).
 

Comment 1 by shend@chromium.org, Jan 20 2018

Owner: shend@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 22 2018

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

commit 79228b03444ea7a864eb341ea929047021ed900b
Author: Darren Shen <shend@chromium.org>
Date: Mon Jan 22 03:57:15 2018

[css-typed-om] Throw exception if serializing unsupported CSSMathValues

Currently, there are some CSSMathValues that can't be serialized because
we have not implemented the calc lv4 spec [1]. This patch makes toString
throw an exception if we try to serialize to a calc value that is not
yet valid in Blink.

We fix the web platform test to match the lv4 spec, but we still leave
the test as .tentative as the spec is still a draft.

[1] https://drafts.csswg.org/css-values-4/#calc-notation

Bug:  803739 
Change-Id: I12253dbfcd58f0f0d8def26368e93e873ddc8323
Reviewed-on: https://chromium-review.googlesource.com/877200
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530804}
[add] https://crrev.com/79228b03444ea7a864eb341ea929047021ed900b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/stylevalue-serialization/cssMathValue.tentative-expected.txt
[modify] https://crrev.com/79228b03444ea7a864eb341ea929047021ed900b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/stylevalue-serialization/cssMathValue.tentative.html
[modify] https://crrev.com/79228b03444ea7a864eb341ea929047021ed900b/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.cpp
[modify] https://crrev.com/79228b03444ea7a864eb341ea929047021ed900b/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h
[modify] https://crrev.com/79228b03444ea7a864eb341ea929047021ed900b/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl

Comment 3 by shend@chromium.org, Jan 22 2018

Status: Fixed (was: Assigned)
Closing this now. Will reopen this if we need to do option C.

Comment 4 by shend@chromium.org, Jan 24 2018

 Issue 803688  has been merged into this issue.

Sign in to add a comment