New issue
Advanced search Search tips

Issue 726147 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Show other hotlists

Hotlists containing this issue:
style-dev-current


Sign in to add a comment

[css-align] 'auto' is invalid for justify-items and should be replaced by 'legacy'

Project Member Reported by jfernan...@igalia.com, May 24 2017

Issue description

The 'auto' value has been renamed to 'legacy' (already supported keyword), so that when used by its own, it will use the inherited value (if any the legacy keyword) or 'normal otherwise.

https://github.com/w3c/csswg-drafts/issues/1318
 
This issue is just about making 'auto' invalid. 
The new 'legacy' behavior will be implemented in  issue #726148 
Components: Blink>CSS
Labels: Hotlist-Interop
Status: Available (was: Untriaged)
Labels: Update-Monthly
Owner: rob.b...@samsung.com
Status: Started (was: Available)
Labels: -Update-Monthly
Rob, are you still working on this ? I'd like to assume this task it otherwise.
Cc: cbiesin...@chromium.org e...@chromium.org ericwilligers@chromium.org
Eric, do you think we need to send an "intent to deprecate and remove" request for this issue ? We actually shipped support for the 'auto' value for the justify-items property, although I doubt it's being used now. 

Only Grid Layout, shipped last year, implements the CSS Box Alignment spec completely; Flexbox has a 'partial implementation', since the new spec was born as a generalization of the alignment properties defined by the old Flexbox spec.

The CSS Box Alignment spec changed the justify-item property syntax defining a new CSS value 'legacy' to replace the previously allowed 'auto' value. Hence, the idea is not only to deprecated and remove 'auto', but to implement the new 'legacy' value (see  bug #726148 )

I'll implement the same change in WebKit [1] and it seems Firefox is already working on this [2]

1- https://bugs.webkit.org/show_bug.cgi?id=172711
2- https://bugzilla.mozilla.org/show_bug.cgi?id=1363875
See https://www.chromium.org/blink/removing-features

If possible when removing functionality, we should give authors at least one release with warnings so they can migrate.

Sometimes this isn't feasible (e.g. changing a property's serialization). If it isn't feasible here, we should explain the situation in the Intent for shipping 726148.

As I commented in  bug #726148 , what I'm trying to do here is a 'replacement' more than a 'removal'. 

If I must follow this approach, I should then send first an intent-to-implement-and-ship for the new 'legacy' CSS value, as an alternative to the feature we are removing here.
I recently had a 'replacement' myself when some properties were renamed. The clearest way to explain it was by including the Ship and the Remove in a single Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/o1C5NzGf9Q0/sbaLF1MbAgAJ


Our code reviewers are checking code quality and test coverage and consistency with communicated Intent; they aren't judging the wisdom of adding to or removing from our public API as that determination should already have been made in an Intent thread.

Summary: [css-align] 'auto' is invalid for justify-items and should be replaced by 'legacy' (was: [css-align] 'auto' is invalid for justify-items)
Owner: jfernan...@igalia.com
Also here, never got around to it sadly, re-assigning.
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/464de2c5b1f00abced98a1a705889046859f5948

commit 464de2c5b1f00abced98a1a705889046859f5948
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Fri Feb 09 00:32:36 2018

[css-align] justfy-items accepts 'legacy' and drops support for 'auto'

The syntax of the 'justify-items' property accepts a new 'legacy' value,
replacing the 'auto' value which is now parsed as invalid.

https://github.com/w3c/csswg-drafts/issues/1318

This change affects also to the 'place-items' shorthand, which doesn't
accept 'auto' and, for the time being, neither 'legacy'.

Link to the intent-to-ship-and-remove request:

https://groups.google.com/a/chromium.org/d/msgid/blink-dev/552753c1-9b2f-bb01-4fed-2ae621f2398e%40igalia.com?utm_medium=email&utm_source=footer

Bug:  726147 ,  726148 
Change-Id: I219de66b813d350fe33f00a1d4369bed8e9a2350
Reviewed-on: https://chromium-review.googlesource.com/903162
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535593}
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/external/wpt/css/css-align/content-distribution/place-content-shorthand-004.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/external/wpt/css/css-align/default-alignment/parse-justify-items-002.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/external/wpt/css/css-align/default-alignment/place-items-shorthand-004.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/fast/alignment/parse-justify-items.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/LayoutTests/fast/alignment/parse-place-items.html
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/CSSParsingUtils.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/CSSParsingUtils.h
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/ComputedStyleUtils.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/longhands/JustifyItemsCustom.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/shorthands/PlaceItemsCustom.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/properties/shorthands/PlaceSelfCustom.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/464de2c5b1f00abced98a1a705889046859f5948/third_party/WebKit/Source/core/style/ComputedStyleConstants.h

Status: Fixed (was: Started)
This issue should be FIXED now.
This seems like it's going to break a bunch of pages. Wouldn't it be better to issue a console warning for a few years before removing 'auto'?
As it was discussed in the intent-to-remove [1] the patch replaces the 'auto' value with 'legacy', which is also the property's default value. Any site using the 'auto' value, explicitly (it was the default value before this change) will get the behavior now provided by 'legacy', which is exactly the same we implemented for 'auto'.

Additionally, the justify-items property is implemented only for CSS Grid Layout and the 'legacy' keyword, is a quite rare/unusual case for this property. Hence, the probability of breaking any site is pretty low. 

Sign in to add a comment