Summary: | [CSS Grid Layout] Enable runtime feature if feature flag is enabled | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Manuel Rego Casasnovas <rego> | ||||||||||||
Component: | CSS | Assignee: | Manuel Rego Casasnovas <rego> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, darin, jfernandez, kling, rniwa, simon.fraser, svillar | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Manuel Rego Casasnovas
2014-03-10 03:28:24 PDT
Created attachment 226292 [details]
Patch
Comment on attachment 226292 [details] Patch Attachment 226292 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5520739989979136 New failing tests: fast/css-grid-layout/grid-disabled-by-default.html Created attachment 226294 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 226299 [details]
Patch
Removing fast/css-grid-layout/grid-disabled-by-default.html as it fails with this patch.
Created attachment 226413 [details]
Patch
Finally keeping layout test but checking that the runtime feature can be disabled.
Comment on attachment 226413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226413&action=review > Source/WebKit2/Shared/WebPreferencesStore.h:105 > +#if ENABLE(CSS_GRID_LAYOUT) > +#define DEFAULT_CSS_GRID_LAYOUT_ENABLED true > +#else > +#define DEFAULT_CSS_GRID_LAYOUT_ENABLED false > +#endif This is only changing WebKit2. What about WebKit1? Created attachment 226957 [details]
Patch
Changing WK1 code too.
(In reply to comment #7) > Created an attachment (id=226957) [details] > Patch > > Changing WK1 code too. Anyway, maybe it would be a better idea to make the runtime feature true directly in WebCore/page/Settings.in. So if you compile with ENBALE_CSS_GRID_LAYOUT the feature will be enabled by default (and you have still the chance to disable the runtime feature if needed). And if you don't compile with ENBALE_CSS_GRID_LAYOUT the feature won't work as expected. If you prefer this kind of change, I can change the patch and do something like that. What do you think? Comment on attachment 226957 [details] Patch Clearing flags on attachment: 226957 Committed r166614: <http://trac.webkit.org/changeset/166614> All reviewed patches have been landed. Closing bug. |