Summary: Web Inspector's Debugger buttons positioned incorrectly, align-items does not appear to be taking affect. This appears to be an issue with the Runtime setting exposed via bug 163432 <http://trac.webkit.org/changeset/207402> which disabled the runtime feature.
Created attachment 291896 [details] [IMAGE] Issue
I feel like this may have been caused by: <https://webkit.org/b/161303> [css-align] Initial values are parsed as invalid for some Alignment properties <https://trac.webkit.org/r205807> It looks like before then there were some valid properties outside of grid for flex box, that might not work unless CSSGridLayout is enabled.
From that change: > The CSS Box Alignment specification defines a new syntax for > align-self/align-items, justify-self/justify-items and > align-content/justify-content CSS properties. This new syntax include new > values, some of them not valid following the old syntax defined in the CSS > Flexible box specification for some of these properties. > > Due to the patch landed for fixing bug #151591 we are now sure that the > parsing logic for the new syntax is only used when the CSS Grid Layout is > enabled, since this new layout model defines its alignment procedures based > on the mentioned CSS properties. However, in testing, it seems I now have to provide: align-content: center; To override the default value of: align-content: flex-start; when CSSGridLayout is disabled. That sounds like a backwards compatibility issue. Is that intentional?
Note that previously the default value of `align-content` was `normal`. That now doesn't appear to be a valid option unless CSSGridLayout is enabled. That at least does appear to be a backwards incompatible change.
I think I know what's the problem, but I'd need to know exactly the expected behavior.
This bug has been already fixed in Blink (see crbug.com/647694) so it's just a matter of porting my patch to WebKit.
Created attachment 291934 [details] Patch
Comment on attachment 291934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291934&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2441 > +#endif Wondering if we could simplify this a bit. What about: CSSValueID valueID = data.position() == ContentPositionNormal ? normalBehaviorValueID : data.position(); #if ENABLE(CSS_GRID_LAYOUT) // Perhaps some comment here explaining that grid layout requires the new alignment behavior if (RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled()) valueID = data.position(); #endif result.get().append(cssValuePool.createValue(valueID); > LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html:1 > +<!doctype html> DOCTYPE in capital
Comment on attachment 291934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291934&action=review >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2441 >> +#endif > > Wondering if we could simplify this a bit. What about: > > CSSValueID valueID = data.position() == ContentPositionNormal ? normalBehaviorValueID : data.position(); > #if ENABLE(CSS_GRID_LAYOUT) > // Perhaps some comment here explaining that grid layout requires the new alignment behavior > if (RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled()) > valueID = data.position(); > #endif > result.get().append(cssValuePool.createValue(valueID); It looks much better, indeed. >> LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html:1 >> +<!doctype html> > > DOCTYPE in capital Done.
Comment on attachment 291934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291934&action=review >>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2441 >>> +#endif >> >> Wondering if we could simplify this a bit. What about: >> >> CSSValueID valueID = data.position() == ContentPositionNormal ? normalBehaviorValueID : data.position(); >> #if ENABLE(CSS_GRID_LAYOUT) >> // Perhaps some comment here explaining that grid layout requires the new alignment behavior >> if (RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled()) >> valueID = data.position(); >> #endif >> result.get().append(cssValuePool.createValue(valueID); > > It looks much better, indeed. Umm, not that easy because ContentPositionNormal is just an enum while normalBehaviorValueID is a CSSValueID. However, there should be a way to perform the simplification you suggests, so I'll do it in the patch for landing.
Created attachment 291955 [details] Patch Patch for landing.
Created attachment 292059 [details] Patch Patch for landing.
Comment on attachment 292059 [details] Patch Clearing flags on attachment: 292059 Committed r207535: <http://trac.webkit.org/changeset/207535>
All reviewed patches have been landed. Closing bug.
Things look much better now! Thanks for the quick fix!
*** Bug 163700 has been marked as a duplicate of this bug. ***
*** Bug 163382 has been marked as a duplicate of this bug. ***