Bug 223141 - Runtime-disabled CSS features still still appear enabled via two-arg CSS.supports()
Summary: Runtime-disabled CSS features still still appear enabled via two-arg CSS.supp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-12 14:15 PST by Sam Sneddon [:gsnedders]
Modified: 2021-03-22 11:57 PDT (History)
10 users (show)

See Also:

Attachments
Patch (10.10 KB, patch)
2021-03-16 11:14 PDT, Simon Fraser (smfr) 		koivisto: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Sneddon [:gsnedders] 2021-03-12 14:15:47 PST 
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
Comment 1 Radar WebKit Bug Importer 2021-03-12 14:16:09 PST 
<rdar://problem/75377891>
Comment 2 Simon Fraser (smfr) 2021-03-16 10:49:40 PDT 
There's a different entry point for the comma separated list version: DOMCSSNamespace::supports(Document& document, const String& property, const String& value)
Comment 3 Simon Fraser (smfr) 2021-03-16 11:14:23 PDT 
Created attachment 423362 [details]
Patch
Comment 4 Antti Koivisto 2021-03-16 11:28:48 PDT 
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
Comment 5 Simon Fraser (smfr) 2021-03-16 14:40:56 PDT 
https://trac.webkit.org/changeset/274520/webkit
Comment 6 Darin Adler 2021-03-16 14:58:09 PDT 
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.
Comment 7 Darin Adler 2021-03-16 14:58:10 PDT 
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.