Bug 97457 – [WK2] TestRunner does not support overridePreference with value "0"

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Dominik Röttsches (drott)]

fast/regions/css-regions-disabled.html disables CSS Regions with an overridePreference call, that doesn't seem to have any effect on EFL WebKit2.

[Chris Dumez]

There seems to be support for this in WebKitTestRunner already. I will debug this once the dependency patch lands.

[Chris Dumez]

The issue is that the current WKTR overridePreference() takes a boolean in argument even though the tests are passing a string. The issue is that "0" and "1" are both evaluated to true boolean value. As a consequence, CSS regions do not get disabled despite the fact that the test calls:
testRunner.overridePreference("WebKitCSSRegionsEnabled", "0");

I will upload a patch soon.

[Tony Chang]

I had the same problem when adding an overridePreference value for CSS Grid Layout.

Then I noticed that some features are set in internals.settings instead of overridePreference.  This is cross platform code rather than needing to modify testRunner.overridePreference in every port.

I think we should try to move more runtime flags into internals.settings.

[Chris Dumez]

Created attachment 165422 [details]
Patch

[Chris Dumez]

(In reply to comment #3)
> I had the same problem when adding an overridePreference value for CSS Grid Layout.
> 
> Then I noticed that some features are set in internals.settings instead of overridePreference.  This is cross platform code rather than needing to modify testRunner.overridePreference in every port.
> 
> I think we should try to move more runtime flags into internals.settings.

In this case, it is a preference that is already present (I'm not adding it). I'm merely fixing an issue with the current implementation of testRunner.overridePreference() for WebKit2. Also note that on WebKit2, we use the same testRunner.overridePreference() implementation for all ports.

Anyway, I think my patch is still valid since it fixes an issue with the existing implementation.

[Alexey Proskuryakov]

Hmm, this is an interesting case. Mac preferences also support "YES", but I don't know how exactly the conversion to boolean works.

[Alexey Proskuryakov]

Comment on attachment 165422 [details]
Patch

Certainly looks like an improvement to me.

[Chris Dumez]

(In reply to comment #6)
> Hmm, this is an interesting case. Mac preferences also support "YES", but I don't know how exactly the conversion to boolean works.

I don't mind adding support for "YES" but there does not seem to be any test case using it. Do you think it is worth adding?

[WebKit Review Bot]

Comment on attachment 165422 [details]
Patch

Clearing flags on attachment: 165422

Committed r129430: <http://trac.webkit.org/changeset/129430>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.