CSS Logical Properties and Values

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

CSS Logical Properties and Values

Oriol Brufau
https://drafts.csswg.org/css-logical-1/ defines properties and values to control layout
through logical, rather than physical, direction and dimension mappings.
 
I have been working on implementing this in Blink, now I'm starting with WebKit.
 
Various properties are already supported but using non-standard names with a -webkit-
prefix. The standard properties have already been shipped in Firefox and Blink, so
I will start with implementing them and converting the prefixed ones into aliases.
This will happen in https://bugs.webkit.org/show_bug.cgi?id=188386, the specific list
of properties is:
 * margin-{block,inline}-{start,end}
 * padding-{block,inline}-{start,end}
 * border-{block,inline}-{start,end}
 * border-{block,inline}-{start,end}-{width,style,color}
 * {block,inline}-size
 * {min,max}-{block,inline}-size
 
After this I will continue with the remaining flow-relative box model properties and the
flow-relative values for existing properties:
 * margin-{block,inline}
 * padding-{block,inline}
 * border-{block,inline}
 * border-{block,inline}-{width,style,color}
 * float: inline-{start,end}
 * clear: inline-{start,end}
 * resize: {block,inline}
This will be behind an experimental flag, like it's being done in Blink. Firefox shipped some.
 
I would like to get your feedback and comments about this topic, if you are fine with the plan
we will start to land patches next week.

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Simon Fraser-6
This plan sounds great! Thanks for working on this.

Simon

On Aug 8, 2018, at 9:53 AM, Oriol Brufau <[hidden email]> wrote:

https://drafts.csswg.org/css-logical-1/ defines properties and values to control layout
through logical, rather than physical, direction and dimension mappings.
 
I have been working on implementing this in Blink, now I'm starting with WebKit.
 
Various properties are already supported but using non-standard names with a -webkit-
prefix. The standard properties have already been shipped in Firefox and Blink, so
I will start with implementing them and converting the prefixed ones into aliases.
This will happen in https://bugs.webkit.org/show_bug.cgi?id=188386, the specific list
of properties is:
 * margin-{block,inline}-{start,end}
 * padding-{block,inline}-{start,end}
 * border-{block,inline}-{start,end}
 * border-{block,inline}-{start,end}-{width,style,color}
 * {block,inline}-size
 * {min,max}-{block,inline}-size
 
After this I will continue with the remaining flow-relative box model properties and the
flow-relative values for existing properties:
 * margin-{block,inline}
 * padding-{block,inline}
 * border-{block,inline}
 * border-{block,inline}-{width,style,color}
 * float: inline-{start,end}
 * clear: inline-{start,end}
 * resize: {block,inline}
This will be behind an experimental flag, like it's being done in Blink. Firefox shipped some.
 
I would like to get your feedback and comments about this topic, if you are fine with the plan
we will start to land patches next week.
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Oriol Brufau
In reply to this post by Oriol Brufau
I have written a patch for adding logical shorthand properties. However,
I'm not much sure about how I can implement them behind a flag.

I tried hiding them behind a run-time flag. But unlike Blink, it seems I
can't do this easily in CSSProperties.json.
Instead, I wrapped the code added in CSSPropertyParser.cpp and
CSSComputedStyleDeclaration.cpp inside conditionals with
`RuntimeEnabledFeatures::sharedFeatures().cssLogicalEnabled()`.
This disables the effects of the properties, but doesn't prevent them
from being exposed via `prop in element.style`.

So I will try a compile flag instead. But there are some Web Platform
Tests for the new properties, and I would like them to pass.
Is there a way to tell the testbot to compile with this flag?

I would appreciate some guidance on the preferred way of doing this.

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Simon Fraser-6
> On Aug 21, 2018, at 5:56 PM, Oriol Brufau <[hidden email]> wrote:
>
> I have written a patch for adding logical shorthand properties. However,
> I'm not much sure about how I can implement them behind a flag.
>
> I tried hiding them behind a run-time flag. But unlike Blink, it seems I
> can't do this easily in CSSProperties.json.
> Instead, I wrapped the code added in CSSPropertyParser.cpp and
> CSSComputedStyleDeclaration.cpp inside conditionals with
> `RuntimeEnabledFeatures::sharedFeatures().cssLogicalEnabled()`.
> This disables the effects of the properties, but doesn't prevent them
> from being exposed via `prop in element.style`.
>
> So I will try a compile flag instead. But there are some Web Platform
> Tests for the new properties, and I would like them to pass.
> Is there a way to tell the testbot to compile with this flag?
>
> I would appreciate some guidance on the preferred way of doing this.

The way we do this this now is via flags on CSSParserContext. See "conicGradientsEnabled" for an example.

Simon
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Oriol Brufau
But as far as I can tell, this only prevents conic gradients from being
parsed.

In my case I want to completely hide some CSS properties. Preventing the
parser from accepting their values is not enough, because they are still
exposed via `prop in element.style`.


El 22/8/18 a les 03:11, Simon Fraser ha escrit:
> The way we do this this now is via flags on CSSParserContext. See "conicGradientsEnabled" for an example.
>
> Simon

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Oriol Brufau
In reply to this post by Simon Fraser-6
In Blink, properties are defined in CSSProperties.json5, there you can
specify a "runtime_flag" property whose value is the name of a runtime flag.

This overrides the IsEnabled method with a check for the runtime flag:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/build/scripts/core/css/properties/templates/css_property_subclass.h.tmpl?rcl=ab3ab1ba020bb36e969a05575a9e52f5a5b44b1a&l=37-39

And then this method is called in various places like
CSSStyleDeclaration::NamedPropertyEnumerator
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/css_style_declaration.cc?rcl=ab3ab1ba020bb36e969a05575a9e52f5a5b44b1a&l=199

Runtime flags can be defined like
https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/runtime_enabled_features.json5?rcl=ab3ab1ba020bb36e969a05575a9e52f5a5b44b1a&l=271-272

I can run chrome or content_shell with
--enable-experimental-web-platform-features in order to enable
experimental flags. Tests are run this way.

El 22/8/18 a les 03:26, Simon Fraser ha escrit:
> Right, sorry. I don't think we have a way to do that.
>
> How does Blink solve this problem?
>
> Simon
>
>

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Michael Catanzaro
In reply to this post by Oriol Brufau
On Tue, Aug 21, 2018 at 7:56 PM, Oriol Brufau <[hidden email]>
wrote:
> So I will try a compile flag instead. But there are some Web Platform
> Tests for the new properties, and I would like them to pass.
> Is there a way to tell the testbot to compile with this flag?

If you absolutely must add a new compile flag... then after adding it
to WebKitFeatures.cmake, you can add enable it like so in
OptionsGTK.cmake:

WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_STREAM PRIVATE
${ENABLE_EXPERIMENTAL_FEATURES})
WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SERVICE_WORKER PRIVATE
${ENABLE_EXPERIMENTAL_FEATURES})
WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEB_RTC PRIVATE
${ENABLE_EXPERIMENTAL_FEATURES})

It has to be done separately for each port that wants the experimental
feature enabled on bots.

This is discouraged, of course. It's better if you don't need a build
flag at all. But that is how you can do it if you can't find another
solution to this CSS properties issue.

Michael

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: CSS Logical Properties and Values

Oriol Brufau
In reply to this post by Oriol Brufau
I tried adding a compile flag set to ENABLE_EXPERIMENTAL_FEATURES,
but not all platforms run tests with this set to true.
So I had to skip the tests in some platforms, which was bad.

Additionally, my patch for logical shorthands is not the only affected,
for example the tests that need ENABLE_CSS3_TEXT are skipped.
https://trac.webkit.org/changeset/228817/webkit#file1

So it became clear that a proper way to disable CSS properties
behind runtime flags is needed. I have added such a feature in
my patch for https://bugs.webkit.org/show_bug.cgi?id=188697

It will suffice to specify a "runtime-flag" in the "codegen-properties"
of the property in Source/WebCore/css/CSSProperties.json.
The value should be the name of a flag in RuntimeEnabledFeatures.
If the flag is disabled, the property won't be exposed, i.e

property in element.style; // false
element.style[property]; // undefined
element.style.getPropertyValue(property); // ""
element.style.setProperty(property, value); // no effect
element.style.all = "inherit"; // won't set `property`
element.style.cssText; // won't include `property`

I have also added some tests about webexposed properties.

Does this approach look good? Please review my patch (only the
parts related with runtime flags, Simon Fraser already approved
the logical shorthands).


El 22/8/18 a les 02:56, Oriol Brufau ha escrit:

> I have written a patch for adding logical shorthand properties. However,
> I'm not much sure about how I can implement them behind a flag.
>
> I tried hiding them behind a run-time flag. But unlike Blink, it seems I
> can't do this easily in CSSProperties.json.
> Instead, I wrapped the code added in CSSPropertyParser.cpp and
> CSSComputedStyleDeclaration.cpp inside conditionals with
> `RuntimeEnabledFeatures::sharedFeatures().cssLogicalEnabled()`.
> This disables the effects of the properties, but doesn't prevent them
> from being exposed via `prop in element.style`.
>
> So I will try a compile flag instead. But there are some Web Platform
> Tests for the new properties, and I would like them to pass.
> Is there a way to tell the testbot to compile with this flag?
>
> I would appreciate some guidance on the preferred way of doing this.
>

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev