Skip to content

Commit db0a169

Browse files
GregableGregable
authored
fix: correct CSS error column numbers when <style> and content are on the same line (#40490)
* fix: correct CSS error column numbers when <style> and content are on the same line Plumb text node position through CdataMatcher to the CSS tokenizer so that column numbers reflect the actual CSS content position, not the opening tag position. Includes test case for same-line style+CSS. Fixes #40424 * fix: use tag line + content column for CSS error positions Use the tag's line number (line_col_) but the content's column (content_line_col) in MatchCss. This preserves correct line numbering for multi-line stylesheets while fixing column offsets when CSS content is on the same line as the <style> tag. Also corrects the expected test output to match actual validator output.
1 parent f45b5ec commit db0a169

3 files changed

Lines changed: 70 additions & 7 deletions

File tree

‎validator/cpp/engine/validator-internal.cc‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1953,16 +1953,21 @@ class CdataMatcher {
19531953
const LineCol& line_col);
19541954

19551955
// Matches |cdata| against what this CdataMatcher expects.
1956+
// |content_line_col| is the position of the actual text content (e.g., the
1957+
// text node inside <style>), which may differ from the opening tag position
1958+
// when the tag and content are on the same line.
19561959
void Match(string_view cdata, Context* context,
1957-
ValidationResult* result) const;
1960+
ValidationResult* result,
1961+
const LineCol& content_line_col) const;
19581962

19591963
private:
19601964
// Matches the provided cdata against a CSS specification. Helper
19611965
// routine for match (see above). |url_bytes| contains the number of bytes
19621966
// in the CSS string which were measured as URLs. In some validation types,
19631967
// these bytes are not counted against byte limits.
19641968
void MatchCss(string_view cdata, const CssSpec& css_spec, int* url_bytes,
1965-
Context* context, ValidationResult* result) const;
1969+
Context* context, ValidationResult* result,
1970+
const LineCol& content_line_col) const;
19661971

19671972
// Matches the provided stylesheet against a MediaQuery specification.
19681973
// Helper routine for MatchCss.
@@ -2896,7 +2901,8 @@ class InvalidDeclVisitor : public htmlparser::css::RuleVisitor {
28962901
};
28972902

28982903
void CdataMatcher::Match(string_view cdata, Context* context,
2899-
ValidationResult* result) const {
2904+
ValidationResult* result,
2905+
const LineCol& content_line_col) const {
29002906
if (!parsed_cdata_spec_) return;
29012907
if (context->Progress(*result).complete) return;
29022908

@@ -2932,7 +2938,8 @@ void CdataMatcher::Match(string_view cdata, Context* context,
29322938
return;
29332939
}
29342940
} else if (cdata_spec.has_css_spec()) {
2935-
MatchCss(cdata, cdata_spec.css_spec(), &url_bytes, context, result);
2941+
MatchCss(cdata, cdata_spec.css_spec(), &url_bytes, context, result,
2942+
content_line_col);
29362943
} else if (cdata_spec.whitespace_only()) {
29372944
static LazyRE2 ws_only = {"^\\s*$"};
29382945
if (!RE2::FullMatch(cdata, *ws_only)) {
@@ -3100,13 +3107,18 @@ void CdataMatcher::MatchSelectors(
31003107

31013108
void CdataMatcher::MatchCss(string_view cdata, const CssSpec& css_spec,
31023109
int* url_bytes, Context* context,
3103-
ValidationResult* result) const {
3110+
ValidationResult* result,
3111+
const LineCol& content_line_col) const {
31043112
vector<unique_ptr<htmlparser::css::ErrorToken>> css_errors;
31053113
vector<unique_ptr<htmlparser::css::ErrorToken>> css_warnings;
31063114
vector<char32_t> codepoints =
31073115
htmlparser::Strings::Utf8ToCodepoints(cdata.data());
3116+
// Use the tag's line (line_col_) to preserve correct line numbering, but
3117+
// use the content's column (content_line_col) so that when the CSS is on
3118+
// the same line as the <style> tag, column offsets are correct. For
3119+
// multi-line content, the first newline resets the column to 0 anyway.
31083120
vector<unique_ptr<htmlparser::css::Token>> tokens = htmlparser::css::Tokenize(
3109-
&codepoints, line_col_.line(), line_col_.col(), &css_errors);
3121+
&codepoints, line_col_.line(), content_line_col.col(), &css_errors);
31103122
unique_ptr<htmlparser::css::Stylesheet> stylesheet =
31113123
htmlparser::css::ParseAStylesheet(
31123124
&tokens, parsed_cdata_spec_->css_parsing_config(), &css_errors);
@@ -5843,9 +5855,19 @@ class Validator {
58435855
const CdataMatcher* cdata_matcher =
58445856
context_.tag_stack().cdata_matcher();
58455857
if (cdata_matcher) {
5858+
LineCol content_lc = context_.line_col();
5859+
if (auto pos = node->FirstChild()->LineColInHtmlSrc();
5860+
pos.has_value()) {
5861+
// LineColInHtmlSrc() returns 1-indexed columns; convert to
5862+
// 0-indexed to match the convention used by the CSS tokenizer.
5863+
int line_no = pos.value().first;
5864+
int col_no = pos.value().second;
5865+
content_lc = LineCol(line_no >= 0 ? line_no : line_no + 1,
5866+
col_no > 0 ? col_no - 1 : col_no);
5867+
}
58465868
cdata_matcher->Match(string_view(node->FirstChild()->Data().data(),
58475869
node->FirstChild()->Data().size()),
5848-
&context_, &result_);
5870+
&context_, &result_, content_lc);
58495871
}
58505872
}
58515873
}

‎validator/testdata/feature_tests/css_error_column_same_line.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!--
2+
Test Description:
3+
Tests that CSS error column numbers are correct when the style tag and CSS
4+
content appear on the same line.
5+
-->
6+
<!doctype html>
7+
<html ⚡>
8+
<head>
9+
<meta charset="utf-8">
10+
<link rel="canonical" href="./regular-html-version.html">
11+
<meta name="viewport" content="width=device-width,minimum-scale=1">
12+
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
13+
<script async src="https://cdn.ampproject.org/v0.js"></script>
14+
<style amp-custom>a{b{c:d}}</style>
15+
</head>
16+
<body>
17+
Hello, world.
18+
</body>
19+
</html>

‎validator/testdata/feature_tests/css_error_column_same_line.out‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
FAIL
2+
| <!--
3+
| Test Description:
4+
| Tests that CSS error column numbers are correct when the style tag and CSS
5+
| content appear on the same line.
6+
| -->
7+
| <!doctype html>
8+
| <html ⚡>
9+
| <head>
10+
| <meta charset="utf-8">
11+
| <link rel="canonical" href="./regular-html-version.html">
12+
| <meta name="viewport" content="width=device-width,minimum-scale=1">
13+
| <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
14+
| <script async src="https://cdn.ampproject.org/v0.js"></script>
15+
| <style amp-custom>a{b{c:d}}</style>
16+
>> ^~~~~~~~~
17+
feature_tests/css_error_column_same_line.html:14:21 CSS syntax error in tag 'style amp-custom' - incomplete declaration.
18+
| </head>
19+
| <body>
20+
| Hello, world.
21+
| </body>
22+
| </html>

0 commit comments

Comments
 (0)