We should support tab-size with units. This will require plumbing a Length through down to TextRun etc. We might want a new Length type which indicates "unitless value with special meaning"
I believe bug 152683 and this bug report are the same. If this is correct, then one of the 2 should be resolved as DUPLICATE. - - - - testing tab-size: calc(10px) http://w3c-test.org/css/css-values/calc-numbers.html https://wpt.fyi/results/css/css-values/calc-numbers.html?label=master
*** Bug 152683 has been marked as a duplicate of this bug. ***
Created attachment 371393 [details] Patch
Created attachment 371395 [details] Add TabSize header to Source/WebCore/Headers.cmake
Comment on attachment 371395 [details] Add TabSize header to Source/WebCore/Headers.cmake Attachment 371395 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12384726 New failing tests: fast/text/whitespace/simple-line-layout-tab-position.html imported/w3c/web-platform-tests/css/css-text/inheritance.html fast/text/simple-line-wordspacing.html css3/tab-size.html imported/w3c/web-platform-tests/css/css-text/parsing/tab-size-valid.html fast/css/tab-size.html
Created attachment 371404 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 371395 [details] Add TabSize header to Source/WebCore/Headers.cmake Attachment 371395 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12384855 New failing tests: fast/text/whitespace/simple-line-layout-tab-position.html imported/w3c/web-platform-tests/css/css-text/inheritance.html fast/text/simple-line-wordspacing.html css3/tab-size.html imported/w3c/web-platform-tests/css/css-text/parsing/tab-size-valid.html fast/css/tab-size.html
Created attachment 371405 [details] Archive of layout-test-results from ews103 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 371395 [details] Add TabSize header to Source/WebCore/Headers.cmake Attachment 371395 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12384964 New failing tests: fast/text/whitespace/simple-line-layout-tab-position.html imported/w3c/web-platform-tests/css/css-text/inheritance.html fast/text/simple-line-wordspacing.html css3/tab-size.html imported/w3c/web-platform-tests/css/css-text/parsing/tab-size-valid.html fast/css/tab-size.html
Created attachment 371411 [details] Archive of layout-test-results from ews112 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 371395 [details] Add TabSize header to Source/WebCore/Headers.cmake Attachment 371395 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12384991 New failing tests: fast/text/whitespace/simple-line-layout-tab-position.html imported/w3c/web-platform-tests/css/css-text/inheritance.html fast/text/simple-line-wordspacing.html css3/tab-size.html imported/w3c/web-platform-tests/css/css-text/parsing/tab-size-valid.html fast/css/tab-size.html
Created attachment 371415 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Created attachment 371460 [details] Update Layout Test And Result
Comment on attachment 371460 [details] Update Layout Test And Result View in context: https://bugs.webkit.org/attachment.cgi?id=371460&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3030 > + return cssValuePool.createValue( > + style.tabSize().getPixelSize(1.0), style.tabSize().isSpaces() ? CSSPrimitiveValue::CSS_NUMBER : CSSPrimitiveValue::CSS_PX); We don't wrap like this in WebKit code. > Source/WebCore/css/StyleBuilderConverter.h:264 > + return TabSize(primitiveValue.floatValue(), true); We prefer enum to bool arguments. > Source/WebCore/css/parser/CSSPropertyParser.cpp:1126 > + RefPtr<CSSPrimitiveValue> tabSize = consumeNumber(range, ValueRangeNonNegative); auto tabSize > Source/WebCore/layout/inlineformatting/text/TextUtil.cpp:63 > + if (tabWidth.getPixelSize(1.0)) Why does passing 1.0 as the spaceWidth make sense here? > Source/WebCore/platform/graphics/TabSize.h:35 > + TabSize(float numOrLength, bool isSpaces = true) isSpaces should be an enum class with two values > Source/WebCore/platform/graphics/TabSize.h:46 > + float getPixelSize(float spaceWidth) const widthInPixels? > Source/WebCore/platform/graphics/TabSize.h:52 > + unsigned m_isSpaces : 1; No point using a bitfield here; the struct will get padded anyway. Just use a bool. > Source/WebCore/platform/graphics/TextRun.h:100 > + void setTabSize(bool, TabSize); Why is TabSize passed by value here, but by reference earlier? > Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp:259 > + if (m_style.tabWidth.getPixelSize(1.0)) That mystery 1.0 again.
Comment on attachment 371460 [details] Update Layout Test And Result View in context: https://bugs.webkit.org/attachment.cgi?id=371460&action=review >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3030 >> + style.tabSize().getPixelSize(1.0), style.tabSize().isSpaces() ? CSSPrimitiveValue::CSS_NUMBER : CSSPrimitiveValue::CSS_PX); > > We don't wrap like this in WebKit code. Ok, I will remove this wrap here. >> Source/WebCore/css/StyleBuilderConverter.h:264 >> + return TabSize(primitiveValue.floatValue(), true); > > We prefer enum to bool arguments. Ok, I will use enum here. >> Source/WebCore/css/parser/CSSPropertyParser.cpp:1126 >> + RefPtr<CSSPrimitiveValue> tabSize = consumeNumber(range, ValueRangeNonNegative); > > auto tabSize Done. >> Source/WebCore/layout/inlineformatting/text/TextUtil.cpp:63 >> + if (tabWidth.getPixelSize(1.0)) > > Why does passing 1.0 as the spaceWidth make sense here? I used this to check if the value TabSize::m_floatValue is non-zero. Then I will add operator bool() const { return m_floatValue; } to TabSize.h instead of call to getPixelSize explicitly. >> Source/WebCore/platform/graphics/TabSize.h:35 >> + TabSize(float numOrLength, bool isSpaces = true) > > isSpaces should be an enum class with two values Done. >> Source/WebCore/platform/graphics/TabSize.h:46 >> + float getPixelSize(float spaceWidth) const > > widthInPixels? Ok, I will change the naming to this one. >> Source/WebCore/platform/graphics/TabSize.h:52 >> + unsigned m_isSpaces : 1; > > No point using a bitfield here; the struct will get padded anyway. Just use a bool. Done. >> Source/WebCore/platform/graphics/TextRun.h:100 >> + void setTabSize(bool, TabSize); > > Why is TabSize passed by value here, but by reference earlier? Ok, I will change these getter/setter from pass by value to pass by reference. >> Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp:259 >> + if (m_style.tabWidth.getPixelSize(1.0)) > > That mystery 1.0 again. Ditto.
Created attachment 371477 [details] Apply Simon's comments
Comment on attachment 371477 [details] Apply Simon's comments View in context: https://bugs.webkit.org/attachment.cgi?id=371477&action=review > Source/WebCore/css/StyleBuilderConverter.h:260 > +inline TabSize StyleBuilderConverter::convertLengthOrTabSpaces(const StyleResolver& styleResolver, const CSSValue& value) I think this should just be called convertTabSize() > Source/WebCore/platform/graphics/TabSize.h:58 > + float m_floatValue; m_value > LayoutTests/css3/tab-size.html:16 > +.int-tab-size { > + tab-size: 9; > +} What about fractional tab sizes: tab-size: 2.5 ?
Comment on attachment 371477 [details] Apply Simon's comments View in context: https://bugs.webkit.org/attachment.cgi?id=371477&action=review >> Source/WebCore/css/StyleBuilderConverter.h:260 >> +inline TabSize StyleBuilderConverter::convertLengthOrTabSpaces(const StyleResolver& styleResolver, const CSSValue& value) > > I think this should just be called convertTabSize() Ok, I will rename it with convertTabSize. >> Source/WebCore/platform/graphics/TabSize.h:58 >> + float m_floatValue; > > m_value Done. >> LayoutTests/css3/tab-size.html:16 >> +} > > What about fractional tab sizes: tab-size: 2.5 ? Ok, I will add a fraction related test entry too.
Created attachment 371566 [details] Patch For Landing
Comment on attachment 371566 [details] Patch For Landing Clearing flags on attachment: 371566 Committed r246193: <https://trac.webkit.org/changeset/246193>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51515096>