-
Notifications
You must be signed in to change notification settings - Fork 668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-text] Consider removing distribute
value for text-justify
from the spec
#7322
Comments
cc @karlcow @miketaylr too |
Note that justification doesn't break pages in the way that, say, a layout bug would, so the compat impact of not having this value is going to be less than breaking/dropping any kind of layout behavior. But it's also very cheap to implement, being just an alias. This value was in a CR-level spec in the past and supported by the most dominant browser of the time, so I'm hesitant to drop it if there's not either a compelling reason or substantial proof that it's not used by some long tail of existing CJK content. https://www.w3.org/TR/2003/CR-css3-text-20030514/#justification-prop |
CC @kojiishi |
Following the discussion in #6156
|
The spec (and relevant WPT) specifies it as a parse-time alias, which while not hard to implement, complicates the code unnecessarily, since WebKit does not have such a mechanism (you need to create a custom parsing method, and can't re-use the fast paths). Compute-time is fine though I don't see the benefit of supporting it. Fwiw, Chrome only implements it compute-time. |
https://bugs.webkit.org/show_bug.cgi?id=229084 <rdar://82177456> Reviewed by Antti Koivisto. This is still disabled by default behind the cssTextJustifyEnabled setting. We don't keep support for the -webkit- prefix, since only Firefox ships text-justify (without the prefix) and it was never enabled in WebKit for macOS/iOS. distribute and inter-character are aliases, with distribute being the legacy one. The spec specifies it as parse-time, but we implement it compute-time (like Chrome). w3c/csswg-drafts#7322 is filed about potentially removing 'distribute' from the spec. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-expected.txt: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-inherited-expected.txt: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-inherited.html: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify.html: Removed. Removed parsing tests redundant with WPT. * LayoutTests/fast/css3-text/css3-text-justify/text-justify-none.html: * LayoutTests/fast/text/text-combine-crash.html: * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/resources/resource-files.json: * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-letter-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-line-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/inheritance-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-justify-computed-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-justify-valid-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/letter-spacing/letter-spacing-bidi-003.xht: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-001.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-002.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-003.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-004.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-005.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-006.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/distribute-alias.tentative-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/distribute-alias.tentative.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-002-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-002.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-003-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-003.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-004-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-004.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-005-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-005.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-006-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-006.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-distribute-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-distribute-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-interpolation-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-interpolation.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/w3c-import.log: * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: Import new WPT from 6aa9a39, remove prefixes automatically added by the importer, and rebaseline tests. * Source/WebCore/animation/CSSPropertyAnimation.cpp: (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): * Source/WebCore/css/CSSComputedStyleDeclaration.cpp: (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/css/CSSPrimitiveValueMappings.h: (WebCore::CSSPrimitiveValue::CSSPrimitiveValue): (WebCore::CSSPrimitiveValue::operator TextJustify const): * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/StyleProperties.cpp: * Source/WebCore/css/parser/CSSParserContext.cpp: (WebCore::CSSParserContext::isPropertyRuntimeDisabled const): * Source/WebCore/css/parser/CSSParserFastPaths.cpp: (WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue): (WebCore::CSSParserFastPaths::isKeywordPropertyID): * Source/WebCore/rendering/LegacyLineLayout.cpp: (WebCore::LegacyLineLayout::textAlignmentForLine const): * Source/WebCore/rendering/style/RenderStyleConstants.h: * Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js: Canonical link: https://commits.webkit.org/251144@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295049 268f45cc-cd09-0410-ab3c-d52691b4dbfc
https://bugs.webkit.org/show_bug.cgi?id=229084 <rdar://82177456> Reviewed by Antti Koivisto. This is still disabled by default behind the cssTextJustifyEnabled setting. We don't keep support for the -webkit- prefix, since only Firefox ships text-justify (without the prefix) and it was never enabled in WebKit for macOS/iOS. distribute and inter-character are aliases, with distribute being the legacy one. The spec specifies it as parse-time, but we implement it compute-time (like Chrome). w3c/csswg-drafts#7322 is filed about potentially removing 'distribute' from the spec. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-expected.txt: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-inherited-expected.txt: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify-inherited.html: Removed. * LayoutTests/fast/css3-text/css3-text-justify/getComputedStyle/getComputedStyle-text-justify.html: Removed. Removed parsing tests redundant with WPT. * LayoutTests/fast/css3-text/css3-text-justify/text-justify-none.html: * LayoutTests/fast/text/text-combine-crash.html: * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/resources/resource-files.json: * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-letter-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-line-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/inheritance-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-justify-computed-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-justify-valid-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/letter-spacing/letter-spacing-bidi-003.xht: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-001.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-002.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-003.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-004.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-005.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-006.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/distribute-alias.tentative-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/distribute-alias.tentative.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-002-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-002.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-003-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-003.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-004-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-004.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-005-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-005.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-006-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-and-trailing-spaces-006.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-distribute-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-distribute-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-character-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-inter-word-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-interpolation-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-interpolation.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/text-justify-none-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-justify/w3c-import.log: * LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: * LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-revert-layer-expected.txt: * LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt: Import new WPT from 6aa9a39, remove prefixes automatically added by the importer, and rebaseline tests. * Source/WebCore/animation/CSSPropertyAnimation.cpp: (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap): * Source/WebCore/css/CSSComputedStyleDeclaration.cpp: (WebCore::ComputedStyleExtractor::valueForPropertyInStyle): * Source/WebCore/css/CSSPrimitiveValueMappings.h: (WebCore::CSSPrimitiveValue::CSSPrimitiveValue): (WebCore::CSSPrimitiveValue::operator TextJustify const): * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/StyleProperties.cpp: * Source/WebCore/css/parser/CSSParserContext.cpp: (WebCore::CSSParserContext::isPropertyRuntimeDisabled const): * Source/WebCore/css/parser/CSSParserFastPaths.cpp: (WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue): (WebCore::CSSParserFastPaths::isKeywordPropertyID): * Source/WebCore/rendering/LegacyLineLayout.cpp: (WebCore::LegacyLineLayout::textAlignmentForLine const): * Source/WebCore/rendering/style/RenderStyleConstants.h: * Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js: Canonical link: https://commits.webkit.org/251144@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295049 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Edge has degraded this when it moved to Chromium. If Edge is fine, I think it's a good sign we can remove this safely. /cc @dlibby- |
Given that (as @fantasai notes above),
I think the bar for removing it should be fairly high; I'm not sure that relatively minor (AIUI) implementation convenience is a sufficient justification. (Does anyone have stats for the number of pages where this value is present? A simple search for "text-justify: distribute" finds a number of CSS-related blog posts, tutorials, etc., that mention it, but whether anyone actually used it is another question...) |
I agree the bar for removing backward compatibilities should be high, but when one browser successfully removed it, the fact should clear the high bar, right? |
The fact that we didn't hear about it doesn't mean that no content was affected. The potential breakage would be relatively minor (cosmetic), rather than making sites unusable. And it would most likely be in non-European locales, where any murmurings from users who notice a change in how content is justified might not reach the ears of the (primarily English-speaking) CSSWG. Can we find out how many pages in Google's index (for example) have this value in their styles? |
HTTPArchive could provide some data I guess? @LeaVerou |
That should be pretty easy! |
@Loirooriol here you go: https://docs.google.com/spreadsheets/d/1VoYVJ5HLN_-wogaFPKaJh46j0qbdm0MkM-oPmcVFEjY/edit#gid=0 |
Thanks! Then it seems strange to me to have a |
Yeah, interesting. Another point to note, though, is that it appears |
It would be interesting to find out which pages contains this property. Coming from a CSS framework? Really used on purpose or just here by default and doing nothing most of the time.
|
This is an interesting data, thanks @LeaVerou and @rviscomi, and thanks @karlcow for pointing out about the framework usages. It looks like
I'm fine not to add the backward compat because the breakage is very minor and not supported by 3 out of 4 browsers for a long time, but if we want, I think a) adding more used values ( |
How do WebKit and Blink not have parse time aliases? It's a matter of turning a consumed value into another, right? |
WebKit does, it's just that you have to create a custom parsing function for the CSS property, you can't go through parsing fast paths (which just checks keywords), which is unfortunate. |
@karlcow let me know if this is what you had in mind
|
|
The CSS Working Group just discussed
The full IRC log of that discussion<fantasai> Topic: [css-text] Consider removing distribute value for text-justify from the spec<fantasai> github: https://github.com//issues/7322 <emilio> fantasai: I don't know if we can resolve on this because discussion has expanded a little bit <emilio> ... issue was a suggestion to remove this value, because Chrome / Safari don't support it, but Firefox does and IE did, and it was in CR in 2003 <emilio> ... so my preference is to not remove it unless it's harmful / complicated <emilio> ... we got some data showing that it is used on some pages <emilio> ... not a lot of them <emilio> ... there's an assertion that there's no compat bugs reported, but pages using this in CJK, which are less likely to complain about it <emilio> ... this doesn't cause a fundamental degradation of the page, justification isn't just what's exactly desired <emilio> ... the main difference is whether you distribute in between letters in english / greek / , but not in CJK (??) <emilio> ... it's something that was standard and implemented by the most widely used engine at the time <emilio> ... so I'd lean against removing it <emilio> ... because people should expect it to continue to work <astearns> +1 to not removing for now <fantasai> emilio: The only thing I'd say is that we changed how this alias worked in the past <fantasai> emilio: now it's a parse-time alias of inter-character <fantasai> emilio: I don't think it matters substantially <fantasai> emilio: given it's a compat alias <fantasai> emilio: I'd rather not remove it, it's cheap to support <fantasai> emilio: as long as you support inter-character <fantasai> emilio: so don't see a strong reason to remove, since aliases are extremely cheap to support <fantasai> fantasai: The only complaint here was that parse-time aliases are inconvenient in some engines <fantasai> fantasai: we could allow either type of alias (parse-time or compute-time), I don't think it matters <fantasai> Rossen_: Any objections to keeping 'distribute' keyword? <fantasai> RESOLVED: Keep 'distribute' keyword alias |
Kinda off-topic, but not sure if you are aware of |
For some engines this is easier; for some it's harder. Spec allows either, since it's a compat alias and it doesn't matter.
For some engines this is easier; for some it's harder. Spec allows either, since it's a compat alias and it doesn't matter.
Hi @nt1m |
Should probably be fine to keep it. |
tested by web-platform-tests/wpt#37692 |
For some engines this is easier; for some it's harder. Spec allows either, since it's a compat alias and it doesn't matter.
For some engines this is easier; for some it's harder. Spec allows either, since it's a compat alias and it doesn't matter.
Currently there is this note in this section:
Looking at browser support for
text-justify
:text-justify
...which leaves Firefox being the only browser supporting the CSS property.
I suggest that we remove this note from the spec, since there is no clear win in web compatibility in supporting this.
The text was updated successfully, but these errors were encountered: