It allows negative values. We probably shouldn't allow it to look like a strike-through because that could change the semantic meaning of the text.
<rdar://problem/45861658>
Created attachment 354165 [details] Patch
Comment on attachment 354165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354165&action=review > Source/WebCore/ChangeLog:11 > + We should just clamp the value so it can't go above the baseline. > + > + We shouldn't do this at parse time because it's totally reasonable for text-underline-position: under to want > + a negative text-underline-offset. Instead, we just do it at used value time. Will the spec describe this behavior? > Source/WebCore/style/InlineTextBoxStyle.cpp:62 > + return fontMetrics.ascent() + std::max(0.0f, underlineOffset.lengthValue()); Slightly prefer std::max<float>(0, underlineOffset.lengthValue()) > Source/WebCore/style/InlineTextBoxStyle.cpp:64 > + return fontMetrics.ascent() + std::max(0.0f, fontMetrics.underlinePosition() + underlineOffset.lengthOr(0)); Ditto.
Committed r237955: <https://trac.webkit.org/changeset/237955>
Comment on attachment 354165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354165&action=review > Source/WebCore/style/InlineTextBoxStyle.cpp:38 > + int gap = std::max<int>(1, ceilf(defaultGap / 2.0)); This involves conversion to floating point and back, and also conversion between two different floating point types. Unnecessarily complex and inefficient: defaultGap is an int we divide it by 2.0, a double, which means converting it to a double first then we call ceilf, which takes a float, which means converting the double to a float then we pass it to std::max<int>, which converts it back to an int Assuming that we don’t care about the behavior for std::numeric_limits<int>::max(), and I think we don’t since the rest of the code is written without any allowance for integer overflow, we can get the same result without involving double or float: int gap = std::max(1, (defaultGap + 1) / 2);
Comment on attachment 354165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354165&action=review > Source/WebCore/style/InlineTextBoxStyle.cpp:83 > + desiredOffset = std::max<float>(desiredOffset, fontMetrics.ascent()); > + return desiredOffset; No need for two lines of code here. We should just return the value; no need to write it back into the local variable first.