Bug 164367 - Experimental features should not be enabled by default
Summary: Experimental features should not be enabled by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-03 08:49 PDT by Michael Catanzaro
Modified: 2016-11-09 15:21 PST (History)
11 users (show)

See Also:


Attachments
Patch (3.32 KB, patch)
2016-11-03 08:52 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (6.25 KB, patch)
2016-11-08 04:09 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2016-11-03 08:49:55 PDT
We have now done three stable WebKitGTK+ releases with a bunch of experimental features enabled by default.

Experimental features must not be enabled by default. If these are desired for Safari Technology Preview, then Safari should enable them, that's what the runtime settings API is for.
Comment 1 Michael Catanzaro 2016-11-03 08:52:43 PDT
Created attachment 293764 [details]
Patch
Comment 2 Michael Catanzaro 2016-11-03 08:53:16 PDT
(Ryosuke, you might want to remove Custom Elements from here if it's ready for prime time?)
Comment 3 Michael Catanzaro 2016-11-03 08:54:58 PDT
(In reply to comment #0)
> Experimental features must not be enabled by default. If these are desired
> for Safari Technology Preview, then Safari should enable them, that's what
> the runtime settings API is for.

To be clear: it's understandable you don't have UI to toggle these yet. You can override the default value in Safari Technology Preview unconditionally in the meantime, but the WebKit project default should be sane to not screw up other ports.
Comment 4 Ryosuke Niwa 2016-11-03 13:34:05 PDT
Comment on attachment 293764 [details]
Patch

I don't think disabling these features by default is okay.

We need to figure out why these experimental features are enabled by default on GTK+ port and fix that instead.
Comment 5 Michael Catanzaro 2016-11-03 13:49:31 PDT
(In reply to comment #4) 
> We need to figure out why these experimental features are enabled by default
> on GTK+ port and fix that instead.

They're enabled because this is the file in which we define the default values of WebKit features. ;)
Comment 6 Brady Eidson 2016-11-05 11:11:15 PDT
I've replied in the email thread on webkit-dev strongly objecting to reverting all features to disabled-by-default.

Even if you somehow convince Ryosuke to r+ this patch, my review is a standing r-.

We should resolve this on webkit-dev.
Comment 7 Ryosuke Niwa 2016-11-05 14:58:10 PDT
Perhaps the defaults need to be defined per port?
Comment 8 Darin Adler 2016-11-05 15:14:24 PDT
It is bad we’ve turned these on in for the GTK port even though the maintainers of that port don’t want them on. Please lets quickly agree to something, even if it’s not rolling back these defaults.

Brady, do you have ideas about what to do short term?
Comment 9 Brady Eidson 2016-11-06 13:38:09 PST
I think a perfectly fine short term solution would be to have a copy of these prefs for non-Apple ports.  There's not that many of them.
Comment 10 Michael Catanzaro 2016-11-08 04:09:48 PST
Created attachment 294153 [details]
Patch
Comment 11 Michael Catanzaro 2016-11-08 04:11:53 PST
Ryosuke, Brady, Darin, what do you think of this approach? (Note that it removes Custom Elements from the experimental features list; is that desired?)

My changelog entry tries to nod towards Brady's desire for an enumerable features API that's not named experimental features, though my patch does not actually address that at all, leaving it as future work for interested developers.
Comment 12 Michael Catanzaro 2016-11-08 04:18:10 PST
In particular, note that I'm trying to avoid port-specific defaults here, so that Apple developers who know best have to think about what the value should be for other ports here. :) It's still possible to define port-specific defaults at the top of the file with all the other port-specific defaults when they're really needed.
Comment 13 Darin Adler 2016-11-08 09:14:56 PST
Comment on attachment 294153 [details]
Patch

Yes, to me this looks like a good step to take now. I’m sure we can make additional improvements and refinements in the future.
Comment 14 WebKit Commit Bot 2016-11-09 15:21:23 PST
Comment on attachment 294153 [details]
Patch

Clearing flags on attachment: 294153

Committed r208495: <http://trac.webkit.org/changeset/208495>
Comment 15 WebKit Commit Bot 2016-11-09 15:21:28 PST
All reviewed patches have been landed.  Closing bug.