WIP CL: https://chromium-review.googlesource.com/c/564279/

I started by trying to copy how the writing-mode property in CSSParserFastPaths.cpp, but it doesn't quite seem to work; when I build Chrome and open devtools, CSS.supports("line-break", "after-white-space") still returns true, and devtools doesn't indicate that after-white-space is not allowed for line-break.

I notice for writing-mode and -webkit-writing-mode in CSSProperties.json5, one is not an alias of the other; there are two entries:

    {
      name: "writing-mode",
      custom_value: true,
      inherited: true,
      default_value: "horizontal-tb",
      field_template: "keyword",
      keywords: ["horizontal-tb", "vertical-rl", "vertical-lr"],
      priority: "High",
      type_name: "WritingMode",
      include_paths: ["platform/text/WritingMode.h"],
    },
    {
      name: "-webkit-writing-mode",
      custom_value: true,
      inherited: true,
      priority: "High",
      type_name: "WritingMode",
    },

So I suspected that perhaps I should to make it so that "-webkit-line-break" and "line-break" are separate properties; but I haven't been able to get this to work :-/ So far when I tried to do that, there are duplicate entries added in gen/blink/core/ComputedStyleBase.h so the build doesn't work.


kojii@, do you have any ideas about what to try next?

Thank you for working on this.

You're right, we'll need to define two properties, not aliases.

I guess you'll also need to set "custom"-something to at least one of them, since otherwise the generator will try to create storage for both. I'm not sure which "custom" to use, but you probably can start with the one used in writing-mode.

So I guess we can rename the existing one to "line-break", and add the entry for "-webkit" should look like:
{
  name: "-webkit-line-break",
  custom_value: true,
  inherited: true,
  type_name: "LineBreak",
},

Eddy, could you advise?

What we want is to have two separate properties, but the same storage, with different valid values, to support non-standard values in -webkit- property.

I once did this using "custom" for writing-mode, but it was before "json5" and I guess we have a lot more functionalities today that I wonder you may be able to advise better one than using "custom".

Should we still use "custom", or is there better way to do this?

+shend and nainar who have been working on this

IIUC you want line-break to be the same as -webkit-line-break except it should reject a value. 

For ComputedStyle, in that case you should generate -webkit-line-break as usual as a keyword. As for line-break don't generate the storage. You can do so by ommiting the field_template parameter. This will allow you to handwrite public accessors that can reject the "after-white-space" value for the line-break property.

Alright!

So I changed it to omit the field_template paramteer, and no additional properties/methods were generated for ComputedStyleBase -- and after that change, everything seems to work as I wanted - line-break seems to be the same as -webkit-line-break except that after-white-space is not a valid value.

In the CL currently (https://chromium-review.googlesource.com/c/564279/), I changed the logic in IsValidKeywordPropertyAndValue but didn't hand-write any other functions. Do you think it might still be necessary to add extra logic for public accessors, and by public accessors do you mean functions like GetLineBreak?

yes by public accessors I mean functions like Get/Set LineBreak. Fair warning functions of that name are generated already because -webkit-line-break is generated as LineBreak as can be seen here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/CSSProperties.json5?q=cssproperties.json&sq=package:chromium&l=3039

The generated code is here: https://cs.chromium.org/chromium/src/out/Debug/gen/blink/core/ComputedStyleBase.h?l=2226. Following up on some comments on the CL itself. 

Scratch that accessors are not needed in ComputedStyle. CL is good to go. LGTMing there. 

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

commit f6a6833d464b96fbc7cc27abe41e411487779ba2
Author: Quinten Yearsley <qyearsley@google.com>
Date: Wed Jul 12 16:24:57 2017

Disable value "after-white-space" for the unprefixed line-break property.

Bug:  738593 
Change-Id: Iafbe191294f825b9d1446c3a51c3f7ce527b9831
Reviewed-on: https://chromium-review.googlesource.com/564279
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485988}
[add] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/css-parser/line-break-values.html
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/virtual/stable/webexposed/css-properties-as-js-properties-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/virtual/stable/webexposed/css-property-listing-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/webexposed/css-properties-as-js-properties-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/LayoutTests/webexposed/css-property-listing-expected.txt
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp
[modify] https://crrev.com/f6a6833d464b96fbc7cc27abe41e411487779ba2/third_party/WebKit/Source/core/frame/UseCounter.cpp

Alright, I believe this is now done, thanks everyone for the help :-)

Yay! Thank you for working on this!