Bug 121875 – Replace CSS3_TEXT build flag with Experimental feature settings

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

[Myles C. Maxfield]

Enabling CSS3 Text on Mac

[Myles C. Maxfield]

Created attachment 212512 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 212512 [details]
Patch

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

> LayoutTests/platform/mac/TestExpectations:906
>  # CSS3 Text support is not yet enabled (needs ENABLE_CSS3_TEXT).
> -webkit.org/b/58491 fast/css3-text/css3-text-decoration
> -webkit.org/b/76173 fast/css3-text/css3-text-align-last
> -webkit.org/b/99945 fast/css3-text/css3-text-justify [ Failure ]
> -webkit.org/b/112755 fast/css3-text/css3-text-indent [ Failure ImageOnlyFailure ]
> +webkit.org/b/112755 fast/css3-text/css3-text-indent/getComputedStyle/getComputedStyle-text-indent.html [ Failure ]

This comment doesn't appear to be true any more. What should it be saying?

[Myles C. Maxfield]

Comment on attachment 212512 [details]
Patch

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

>> LayoutTests/platform/mac/TestExpectations:906
>> +webkit.org/b/112755 fast/css3-text/css3-text-indent/getComputedStyle/getComputedStyle-text-indent.html [ Failure ]
> 
> This comment doesn't appear to be true any more. What should it be saying?

Oops, I missed the comment. I'll upload a modified version

[Myles C. Maxfield]

Created attachment 212596 [details]
Patch

[Tim Horton]

Comment on attachment 212596 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

Gotta fix this line. Remove it or explain why (I think there are no new tests because the tests already exist and you're turning them on? You should say that.

> Tools/Scripts/webkitperl/FeatureList.pm:201
> +      define => "ENABLE_CSS3_TEXT", default => (isAppleMacWebKit() || isEfl() || isGtk()), value => \$css3TextSupport },

Does it work on Windows/should we turn it on there too? Most webby features like this we try to keep in sync, I think.

> LayoutTests/platform/mac/TestExpectations:905
> +webkit.org/b/112755 fast/css3-text/css3-text-indent/getComputedStyle/getComputedStyle-text-indent.html [ Failure ]

Why? Does it just need new expectations?

[Myles C. Maxfield]

Reopened because we want to do this

[Brady Eidson]

Comment on attachment 212596 [details]
Patch

Assuming that patches for review since 2013 are stale, r-

[Ebrahim Byagowi]

BUMP

[Tim Nguyen (:ntim)]

I don't think we should have a build flag, some more tailored runtime settings would be much more preferable.

[Kiet Ho]

Pull request: https://github.com/WebKit/WebKit/pull/993

[EWS]

Committed r295010 (251105@main): <https://commits.webkit.org/251105@main>

Reviewed commits have been landed. Closing PR #993 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/94083925>