Bug 223141 – Runtime-disabled CSS features still still appear enabled via two-arg CSS.supports()

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Sam Sneddon [:gsnedders]]

bug 222280 fixed:

> CSS.supports("overscroll-behavior: inherit")

< true

however, even after that:

> CSS.supports("overscroll-behavior: inherit")

< false

> CSS.supports("overscroll-behavior", "inherit")

< true

[Radar WebKit Bug Importer]

<rdar://problem/75377891>

[Simon Fraser (smfr)]

There's a different entry point for the comma separated list version: DOMCSSNamespace::supports(Document& document, const String& property, const String& value)

[Simon Fraser (smfr)]

Created attachment 423362 [details]
Patch

[Antti Koivisto]

Comment on attachment 423362 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423362&action=review

> Source/WebCore/css/DOMCSSNamespace.cpp:67
>  
> +    CSSParserContext parserContext(document);
> +    if (parserContext.isPropertyRuntimeDisabled(propertyID))
> +        propertyID = CSSPropertyInvalid;
> +
>      if (propertyID == CSSPropertyInvalid)
>          return false;

You can just return false in the first branch

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/274520/webkit

[Darin Adler]

Comment on attachment 423362 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423362&action=review

Tiny clean-up suggestions on moved code.

> Source/WebCore/css/parser/CSSParserContext.cpp:161
> +#if ENABLE(TEXT_AUTOSIZING)
> +    case CSSPropertyWebkitTextSizeAdjust:
> +#if !PLATFORM(IOS_FAMILY)
> +        return !textAutosizingEnabled;
> +#endif
> +        return false;
> +#endif // ENABLE(TEXT_AUTOSIZING)

This can be written in a less strange way:

#if ENABLE(TEXT_AUTOSIZING) && !PLATFORM(IOS_FAMILY)
    case CSSPropertyWebkitTextSizeAdjust:
        return !textAutosizingEnabled;
#endif

No need for the nested #if and double return statements.

> Source/WebCore/css/parser/CSSParserContext.cpp:169
> +    return false;

This unreachable code should be removed.

[Darin Adler]

Comment on attachment 423362 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423362&action=review

Tiny clean-up suggestions on moved code.

> Source/WebCore/css/parser/CSSParserContext.cpp:161
> +#if ENABLE(TEXT_AUTOSIZING)
> +    case CSSPropertyWebkitTextSizeAdjust:
> +#if !PLATFORM(IOS_FAMILY)
> +        return !textAutosizingEnabled;
> +#endif
> +        return false;
> +#endif // ENABLE(TEXT_AUTOSIZING)

This can be written in a less strange way:

#if ENABLE(TEXT_AUTOSIZING) && !PLATFORM(IOS_FAMILY)
    case CSSPropertyWebkitTextSizeAdjust:
        return !textAutosizingEnabled;
#endif

No need for the nested #if and double return statements.

> Source/WebCore/css/parser/CSSParserContext.cpp:169
> +    return false;

This unreachable code should be removed.

[Simon Fraser (smfr)]

Followups in https://trac.webkit.org/changeset/274725/webkit and https://trac.webkit.org/changeset/274756/webkit