Summary: | calc() serialization doesn't match the spec | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||
Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dino, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jfernandez, koivisto, macpherson, menard, rego, simon.fraser, svillar, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Safari Technology Preview | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2019-10-25 18:34:28 PDT
Have a mostly-working patch for this. Created attachment 384721 [details]
Patch
Comment on attachment 384721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384721&action=review > Source/WebCore/css/CSSCalculationValue.cpp:556 > + double computeLengthPx(const CSSToLengthConversionData& conversionData) const final { return 1.0 / m_child->computeLengthPx(conversionData); } Could conversionData ever provide a 0? > Source/WebCore/css/CSSCalculationValue.cpp:1026 > + // If nodes contains any dimensions, remove them from nodes, sort them by their units, ordered ASCII case-insensitively, and append them to ret. This comment isn't what you're doing here though right? You're just sorting by ASCII. > Source/WebCore/css/CSSCalculationValue.cpp:1329 > Vector<double> doubleValues; > - for (auto& child : m_children) > - doubleValues.append(child->doubleValue()); > + doubleValues.reserveInitialCapacity(m_children.size()); > + bool allowNumbers = calcOperator() == CalcOperator::Multiply; > + for (auto& child : m_children) { > + CSSUnitType childType = unitType; > + if (allowNumbers && unitType != CSSUnitType::CSS_NUMBER && child->primitiveType() == CSSUnitType::CSS_NUMBER) > + childType = CSSUnitType::CSS_NUMBER; > + doubleValues.uncheckedAppend(child->doubleValue(childType)); > + } You could do evaluate(m_children.map([&] (auto& child) { ... return child->doubleValue(childType); })) and avoid the local variable + reserveInitialCapacity. > Source/WebCore/css/CSSCalculationValue.cpp:1339 > return evaluate(doubleValues); Ditto. > Source/WebCore/css/CSSCalculationValue.cpp:1873 > + return values; Another place where you could .map() > Source/WebCore/platform/CalculationValue.cpp:253 > - case CalcOperator::Min: ts << "max"; break; > - case CalcOperator::Max: ts << "min"; break; > + case CalcOperator::Min: ts << "min"; break; > + case CalcOperator::Max: ts << "max"; break; Wow. Test cleanup in https://trac.webkit.org/changeset/253089/webkit |