New issue
Advanced search Search tips

Issue 789370 link

Starred by 1 user
Status: Fixed
Owner:
shend@chromium.org
Closed: Dec 2017
Cc:
style-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 545318



Sign in to add a comment

[css-typed-om] CSSTransformValues should accept not just CSSUnitValues.

Project Member Reported by shend@chromium.org, Nov 29 2017 
Currently, CSSTransformValue subclasses only take CSSUnitValues, but not CSSMathValues that simplify to a valid CSSUnitValue. For example, CSSTranslation [1] accepts <length-percentage>s, so it should not only accept things like CSS.px(1), but also new CSSMathSum(CSS.px(1), CSS.px(1)).

The logic for matching against things like <length> are already in the codebase, so this should be straightforward.

[1] https://drafts.css-houdini.org/css-typed-om-1/#dom-csstranslation-csstranslation

Comment 2 by shend@chromium.org, Dec 6 2017

Cc: -shend@chromium.org
Owner: shend@chromium.org
Status: Assigned (was: Available)

Comment 3 by nainar@chromium.org, Dec 6 2017

Labels: -Update-Monthly
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 13 2017 

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

commit fb36e8cef3f23bc768c31b6b67840173a6274fca
Author: Darren Shen <shend@chromium.org>
Date: Wed Dec 13 07:09:47 2017

[css-typed-om] Allow transform values to accept CSSMathValues too.

Currently CSSTranslate, CSSRotate and CSSScale do not accept calc values
(CSSMathValues) because it requires type checking the calc value to see
if they match the right type [1]. Since type checking and matching are
now implemented, we have all the machinery to accept CSSMathValues too.

This patch uses CSSNumericValueType.Matches* functions to check if
the CSSNumericValues passed in resolve to the right type.

We also (re)moved some redundant DCHECKs.

We will have a follow up patch for CSSSkew and CSSPerspective.

Note that percents are not allowed in CSSTranslation (spec needs edit):
https://github.com/w3c/css-houdini-drafts/issues/530

[1] https://drafts.css-houdini.org/css-typed-om-1/#cssnumericvalue-match

Specs:
https://drafts.css-houdini.org/css-typed-om-1/#dom-csstranslation-csstranslation
https://drafts.css-houdini.org/css-typed-om-1/#dom-cssrotation-cssrotation
https://drafts.css-houdini.org/css-typed-om-1/#dom-cssrotation-cssrotation-x-y-z-angle
https://drafts.css-houdini.org/css-typed-om-1/#dom-cssscale-cssscale

Bug:  789370 
Change-Id: I6f538a3f07b4c58eb15c9f059c39751e66f64f2f
Reviewed-on: https://chromium-review.googlesource.com/812364
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: meade_UTC10 <meade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523706}
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/LayoutTests/typedcssom/inlinestyle/properties/transform.html
[delete] https://crrev.com/1921bb351242e3ada575cc85858957f9c56106a8/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssRotation-expected.txt
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssRotation.html
[delete] https://crrev.com/1921bb351242e3ada575cc85858957f9c56106a8/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssScale-expected.txt
[delete] https://crrev.com/1921bb351242e3ada575cc85858957f9c56106a8/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssTranslation-expected.txt
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssTranslation.html
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSRotation.cpp
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSRotation.h
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSScale.cpp
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSScale.h
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSTranslation.cpp
[modify] https://crrev.com/fb36e8cef3f23bc768c31b6b67840173a6274fca/third_party/WebKit/Source/core/css/cssom/CSSTranslation.h
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 13 2017 

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

commit 9f8575b7bdf5bbd784e91bd6d6c4e9a722dd5014
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Wed Dec 13 09:46:47 2017

[jumbo] make IsValidCoordinate functions into static members

This should avoid the following jumbo build error:

./../../third_party/WebKit/Source/core/css/cssom/CSSScale.cpp:14:6: error: redefinition of 'IsValidCoordinate'
bool IsValidCoordinate(CSSNumericValue* value) {
     ^
./../../third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp:15:6: note: previous definition is here
bool IsValidCoordinate(CSSNumericValue* v) {
     ^

BUG= 789370 

Change-Id: I13f1540c9969d4ff34420fcfda831687c4580555
Reviewed-on: https://chromium-review.googlesource.com/822199
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Cr-Commit-Position: refs/heads/master@{#523732}
[modify] https://crrev.com/9f8575b7bdf5bbd784e91bd6d6c4e9a722dd5014/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp
[modify] https://crrev.com/9f8575b7bdf5bbd784e91bd6d6c4e9a722dd5014/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h
[modify] https://crrev.com/9f8575b7bdf5bbd784e91bd6d6c4e9a722dd5014/third_party/WebKit/Source/core/css/cssom/CSSScale.cpp
[modify] https://crrev.com/9f8575b7bdf5bbd784e91bd6d6c4e9a722dd5014/third_party/WebKit/Source/core/css/cssom/CSSScale.h
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 15 2017 

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

commit bdfd3647218dc80d4d5119c60b4d3e745182ef9a
Author: Darren Shen <shend@chromium.org>
Date: Fri Dec 15 06:27:12 2017

[css-typed-om] Allow CSSSkew and CSSPerspective to accept CSSMathValues.

Currently CSSSkew and CSSPerspective do not accept calc values
(CSSMathValues) because it requires type checking the calc value to see
if they match the right type. Since type checking and matching are
now implemented, we have all the machinery to accept CSSMathValues too.

This patch uses CSSNumericValueType.Matches* functions to check if
the CSSNumericValues passed in resolve to the right type.

We also (re)moved some DCHECKs.

We also deelted redundant tests in inlinestyle/properties/transform.html

Spec:
https://drafts.css-houdini.org/css-typed-om-1/#dom-cssperspective-cssperspective
https://drafts.css-houdini.org/css-typed-om-1/#dom-cssskew-cssskew

Bug:  789370 
Change-Id: Ice30511b022885004a3a13cca21c8316dd82a271
Reviewed-on: https://chromium-review.googlesource.com/828001
Reviewed-by: meade_UTC10 <meade@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524342}
[modify] https://crrev.com/bdfd3647218dc80d4d5119c60b4d3e745182ef9a/third_party/WebKit/LayoutTests/typedcssom/inlinestyle/properties/transform.html
[delete] https://crrev.com/be9eef8a4e0d7c37a65c0e840fd1b0ec2026538a/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssPerspective-expected.txt
[delete] https://crrev.com/be9eef8a4e0d7c37a65c0e840fd1b0ec2026538a/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssSkew-expected.txt
[modify] https://crrev.com/bdfd3647218dc80d4d5119c60b4d3e745182ef9a/third_party/WebKit/Source/core/css/cssom/CSSPerspective.cpp
[modify] https://crrev.com/bdfd3647218dc80d4d5119c60b4d3e745182ef9a/third_party/WebKit/Source/core/css/cssom/CSSPerspective.h
[modify] https://crrev.com/bdfd3647218dc80d4d5119c60b4d3e745182ef9a/third_party/WebKit/Source/core/css/cssom/CSSSkew.cpp
[modify] https://crrev.com/bdfd3647218dc80d4d5119c60b4d3e745182ef9a/third_party/WebKit/Source/core/css/cssom/CSSSkew.h

Comment 7 by shend@chromium.org, Dec 19 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment