Skip to content

Commit

Permalink
Supply the correct original text to media queries.
Browse files Browse the repository at this point in the history 
We solve this by having a special mode in CSSTokenizer that also stores
the offset of each token, and then propagating that offset list around
with the original string, so that we can recover the original text from
any given token range.

This affects style queries, which we can (in a future patch) then
compare as strings instead of as tokens, without introducing new
issues.

Change-Id: I49c58aacd45efa22689c724489f0e46278349a6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4345516
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118602}
  • Loading branch information
Steinar H. Gunderson authored and Chromium LUCI CQ committed Mar 17, 2023
1 parent bb06909 commit d1c91c6
Show file tree
Hide file tree
Showing 18 changed files with 264 additions and 98 deletions.
5 changes: 3 additions & 2 deletions third_party/blink/renderer/core/css/media_feature_overrides.cc
View file
Expand Up @@ -80,8 +80,9 @@ absl::optional<ForcedColors> ConvertForcedColors(
void MediaFeatureOverrides::SetOverride(const AtomicString& feature,
const String& value_string) {
CSSTokenizer tokenizer(value_string);
const auto tokens = tokenizer.TokenizeToEOF();
auto [tokens, raw_offsets] = tokenizer.TokenizeToEOFWithOffsets();
CSSParserTokenRange range(tokens);
CSSParserTokenOffsets offsets(tokens, std::move(raw_offsets), value_string);

// TODO(xiaochengh): This is a fake CSSParserContext that only passes
// down the CSSParserMode. Plumb the real CSSParserContext through, so that
Expand All @@ -98,7 +99,7 @@ void MediaFeatureOverrides::SetOverride(const AtomicString& feature,
// Document to get the ExecutionContext so the extra parameter should be
// removed.
MediaQueryExpBounds bounds =
MediaQueryExp::Create(feature, range, *fake_context).Bounds();
MediaQueryExp::Create(feature, range, offsets, *fake_context).Bounds();
DCHECK(!bounds.left.IsValid());
MediaQueryExpValue value = bounds.right.value;

Expand Down
9 changes: 6 additions & 3 deletions third_party/blink/renderer/core/css/media_query_evaluator_test.cc
View file
Expand Up @@ -407,10 +407,13 @@ void TestMQEvaluator(MediaQueryEvaluatorTestCase* test_cases,
if (String(test_cases[i].input).empty()) {
query_set = MediaQuerySet::Create();
} else {
StringView str(test_cases[i].input);
CSSTokenizer tokenizer(StringView(test_cases[i].input));
auto [tokens, offsets] = tokenizer.TokenizeToEOFWithOffsets();
query_set = MediaQueryParser::ParseMediaQuerySetInMode(
CSSParserTokenRange(
CSSTokenizer(StringView(test_cases[i].input)).TokenizeToEOF()),
mode, nullptr);
CSSParserTokenRange(tokens),
CSSParserTokenOffsets(tokens, std::move(offsets), str), mode,
nullptr);
}
EXPECT_EQ(test_cases[i].output, media_query_evaluator.Eval(*query_set))
<< "Query: " << test_cases[i].input;
Expand Down
16 changes: 8 additions & 8 deletions third_party/blink/renderer/core/css/media_query_exp.cc
View file
Expand Up @@ -337,9 +337,11 @@ MediaQueryExp::MediaQueryExp(const String& media_feature,

MediaQueryExp MediaQueryExp::Create(const String& media_feature,
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const CSSParserContext& context) {
String feature = AttemptStaticStringCreation(media_feature);
if (auto value = MediaQueryExpValue::Consume(feature, range, context)) {
if (auto value =
MediaQueryExpValue::Consume(feature, range, offsets, context)) {
return MediaQueryExp(feature, *value);
}
return Invalid();
Expand All @@ -348,17 +350,15 @@ MediaQueryExp MediaQueryExp::Create(const String& media_feature,
absl::optional<MediaQueryExpValue> MediaQueryExpValue::Consume(
const String& media_feature,
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const CSSParserContext& context) {
CSSParserContext::ParserModeOverridingScope scope(context, kHTMLStandardMode);

if (CSSVariableParser::IsValidVariableName(media_feature)) {
// CSSParserTokenRange doesn't store precise location information about
// where each token started or ended, so we don't have the actual original
// string. However, for media queries, it's not a huge issue if we get
// normalized whitespace etc., so we work around it by creating
// a fake “original text” by serializing the tokens back.
String serialized = range.Serialize();
CSSTokenizedValue tokenized_value{range, serialized};
base::span span = range.RemainingSpan();
StringView original_string =
offsets.StringForTokens(span.data(), span.data() + span.size());
CSSTokenizedValue tokenized_value{range, original_string};
CSSParserImpl::RemoveImportantAnnotationIfPresent(tokenized_value);
if (const CSSValue* value =
CSSVariableParser::ParseDeclarationIncludingCSSWide(
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/css/media_query_exp.h
View file
Expand Up @@ -43,6 +43,7 @@ namespace blink {

class CSSParserContext;
class CSSParserTokenRange;
class CSSParserTokenOffsets;

class CORE_EXPORT MediaQueryExpValue {
DISALLOW_NEW();
Expand Down Expand Up @@ -141,6 +142,7 @@ class CORE_EXPORT MediaQueryExpValue {
static absl::optional<MediaQueryExpValue> Consume(
const String& lower_media_feature,
CSSParserTokenRange&,
const CSSParserTokenOffsets&,
const CSSParserContext&);

private:
Expand Down Expand Up @@ -257,6 +259,7 @@ class CORE_EXPORT MediaQueryExp {
// Returns an invalid MediaQueryExp if the arguments are invalid.
static MediaQueryExp Create(const String& media_feature,
CSSParserTokenRange&,
const CSSParserTokenOffsets&,
const CSSParserContext&);
static MediaQueryExp Create(const String& media_feature,
const MediaQueryExpBounds&);
Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/css/media_query_set_test.cc
View file
Expand Up @@ -38,9 +38,9 @@ static void TestMediaQuery(const char* input,
actual.Append(", ");
}
if (output) {
ASSERT_EQ(output, actual.ToString());
ASSERT_EQ(String(output), actual.ToString());
} else {
ASSERT_EQ(input, actual.ToString());
ASSERT_EQ(String(input), actual.ToString());
}
}

Expand Down
46 changes: 29 additions & 17 deletions third_party/blink/renderer/core/css/parser/container_query_parser.cc
View file
Expand Up @@ -113,15 +113,17 @@ ContainerQueryParser::ContainerQueryParser(const CSSParserContext& context)
MediaQueryParser::SyntaxLevel::kLevel4) {}

const MediaQueryExpNode* ContainerQueryParser::ParseCondition(String value) {
auto tokens = CSSTokenizer(value).TokenizeToEOF();
auto [tokens, raw_offsets] = CSSTokenizer(value).TokenizeToEOFWithOffsets();
CSSParserTokenRange range(tokens);
return ParseCondition(range);
CSSParserTokenOffsets offsets(tokens, std::move(raw_offsets), value);
return ParseCondition(range, offsets);
}

const MediaQueryExpNode* ContainerQueryParser::ParseCondition(
CSSParserTokenRange range) {
CSSParserTokenRange range,
const CSSParserTokenOffsets& offsets) {
range.ConsumeWhitespace();
const MediaQueryExpNode* node = ConsumeContainerCondition(range);
const MediaQueryExpNode* node = ConsumeContainerCondition(range, offsets);
if (!range.AtEnd()) {
return nullptr;
}
Expand All @@ -133,7 +135,8 @@ const MediaQueryExpNode* ContainerQueryParser::ParseCondition(
// | style( <style-query> )
// | <general-enclosed>
const MediaQueryExpNode* ContainerQueryParser::ConsumeQueryInParens(
CSSParserTokenRange& range) {
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets) {
CSSParserTokenRange original_range = range;

if (range.Peek().GetType() == kLeftParenthesisToken) {
Expand All @@ -144,14 +147,16 @@ const MediaQueryExpNode* ContainerQueryParser::ConsumeQueryInParens(

CSSParserTokenRange original_block = block;
// <size-feature>
const MediaQueryExpNode* query = ConsumeFeature(block, SizeFeatureSet());
const MediaQueryExpNode* query =
ConsumeFeature(block, offsets, SizeFeatureSet());
if (query && block.AtEnd()) {
return MediaQueryExpNode::Nested(query);
}
block = original_block;

// <container-condition>
const MediaQueryExpNode* condition = ConsumeContainerCondition(block);
const MediaQueryExpNode* condition =
ConsumeContainerCondition(block, offsets);
if (condition && block.AtEnd()) {
return MediaQueryExpNode::Nested(condition);
}
Expand All @@ -164,7 +169,7 @@ const MediaQueryExpNode* ContainerQueryParser::ConsumeQueryInParens(
range.ConsumeWhitespace();

if (const MediaQueryExpNode* query =
ConsumeFeatureQuery(block, StyleFeatureSet())) {
ConsumeFeatureQuery(block, offsets, StyleFeatureSet())) {
return MediaQueryExpNode::Function(query, "style");
}
}
Expand All @@ -175,26 +180,29 @@ const MediaQueryExpNode* ContainerQueryParser::ConsumeQueryInParens(
}

const MediaQueryExpNode* ContainerQueryParser::ConsumeContainerCondition(
CSSParserTokenRange& range) {
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets) {
return ConsumeNotAndOr(
[this](CSSParserTokenRange& range) {
return this->ConsumeQueryInParens(range);
[this, offsets](CSSParserTokenRange& range) {
return this->ConsumeQueryInParens(range, offsets);
},
range);
}

const MediaQueryExpNode* ContainerQueryParser::ConsumeFeatureQuery(
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const FeatureSet& feature_set) {
CSSParserTokenRange original_range = range;

if (const MediaQueryExpNode* feature = ConsumeFeature(range, feature_set)) {
if (const MediaQueryExpNode* feature =
ConsumeFeature(range, offsets, feature_set)) {
return feature;
}
range = original_range;

if (const MediaQueryExpNode* node =
ConsumeFeatureCondition(range, feature_set)) {
ConsumeFeatureCondition(range, offsets, feature_set)) {
return node;
}

Expand All @@ -203,14 +211,16 @@ const MediaQueryExpNode* ContainerQueryParser::ConsumeFeatureQuery(

const MediaQueryExpNode* ContainerQueryParser::ConsumeFeatureQueryInParens(
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const FeatureSet& feature_set) {
CSSParserTokenRange original_range = range;

if (range.Peek().GetType() == kLeftParenthesisToken) {
auto block = range.ConsumeBlock();
block.ConsumeWhitespace();
range.ConsumeWhitespace();
const MediaQueryExpNode* query = ConsumeFeatureQuery(block, feature_set);
const MediaQueryExpNode* query =
ConsumeFeatureQuery(block, offsets, feature_set);
if (query && block.AtEnd()) {
return MediaQueryExpNode::Nested(query);
}
Expand All @@ -222,18 +232,20 @@ const MediaQueryExpNode* ContainerQueryParser::ConsumeFeatureQueryInParens(

const MediaQueryExpNode* ContainerQueryParser::ConsumeFeatureCondition(
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const FeatureSet& feature_set) {
return ConsumeNotAndOr(
[this, &feature_set](CSSParserTokenRange& range) {
return this->ConsumeFeatureQueryInParens(range, feature_set);
[this, &offsets, &feature_set](CSSParserTokenRange& range) {
return this->ConsumeFeatureQueryInParens(range, offsets, feature_set);
},
range);
}

const MediaQueryExpNode* ContainerQueryParser::ConsumeFeature(
CSSParserTokenRange& range,
const CSSParserTokenOffsets& offsets,
const FeatureSet& feature_set) {
return media_query_parser_.ConsumeFeature(range, feature_set);
return media_query_parser_.ConsumeFeature(range, offsets, feature_set);
}

} // namespace blink
30 changes: 21 additions & 9 deletions third_party/blink/renderer/core/css/parser/container_query_parser.h
View file
Expand Up @@ -24,22 +24,34 @@ class CORE_EXPORT ContainerQueryParser {

// https://drafts.csswg.org/css-contain-3/#typedef-container-condition
const MediaQueryExpNode* ParseCondition(String);
const MediaQueryExpNode* ParseCondition(CSSParserTokenRange);
const MediaQueryExpNode* ParseCondition(CSSParserTokenRange,
const CSSParserTokenOffsets&);

private:
friend class ContainerQueryParserTest;

using FeatureSet = MediaQueryParser::FeatureSet;

const MediaQueryExpNode* ConsumeQueryInParens(CSSParserTokenRange&);
const MediaQueryExpNode* ConsumeContainerCondition(CSSParserTokenRange&);
const MediaQueryExpNode* ConsumeFeatureQuery(CSSParserTokenRange&,
const FeatureSet&);
const MediaQueryExpNode* ConsumeFeatureQueryInParens(CSSParserTokenRange&,
const FeatureSet&);
const MediaQueryExpNode* ConsumeFeatureCondition(CSSParserTokenRange&,
const FeatureSet&);
const MediaQueryExpNode* ConsumeQueryInParens(
CSSParserTokenRange&,
const CSSParserTokenOffsets& offsets);
const MediaQueryExpNode* ConsumeContainerCondition(
CSSParserTokenRange&,
const CSSParserTokenOffsets&);
const MediaQueryExpNode* ConsumeFeatureQuery(
CSSParserTokenRange&,
const CSSParserTokenOffsets& offsets,
const FeatureSet&);
const MediaQueryExpNode* ConsumeFeatureQueryInParens(
CSSParserTokenRange&,
const CSSParserTokenOffsets&,
const FeatureSet&);
const MediaQueryExpNode* ConsumeFeatureCondition(
CSSParserTokenRange&,
const CSSParserTokenOffsets& offsets,
const FeatureSet&);
const MediaQueryExpNode* ConsumeFeature(CSSParserTokenRange&,
const CSSParserTokenOffsets& offsets,
const FeatureSet&);

const CSSParserContext& context_;
Expand Down
8 changes: 5 additions & 3 deletions third_party/blink/renderer/core/css/parser/container_query_parser_test.cc
View file
Expand Up @@ -48,11 +48,13 @@ class ContainerQueryParserTest : public PageTestBase {
// E.g. https://drafts.csswg.org/css-contain-3/#typedef-style-query
String ParseFeatureQuery(String feature_query) {
const auto* context = MakeGarbageCollected<CSSParserContext>(GetDocument());
Vector<CSSParserToken, 32> tokens =
CSSTokenizer(feature_query).TokenizeToEOF();
auto [tokens, raw_offsets] =
CSSTokenizer(feature_query).TokenizeToEOFWithOffsets();
CSSParserTokenRange range(tokens);
CSSParserTokenOffsets offsets(tokens, std::move(raw_offsets),
feature_query);
const MediaQueryExpNode* node =
ContainerQueryParser(*context).ConsumeFeatureQuery(range,
ContainerQueryParser(*context).ConsumeFeatureQuery(range, offsets,
TestFeatureSet());
if (!node || !range.AtEnd()) {
return g_null_atom;
Expand Down
33 changes: 28 additions & 5 deletions third_party/blink/renderer/core/css/parser/css_parser_impl.cc
View file
Expand Up @@ -876,6 +876,15 @@ StyleRuleCharset* CSSParserImpl::ConsumeCharsetRule(
return MakeGarbageCollected<StyleRuleCharset>();
}

// We need the token offsets for MediaQueryParser, so re-parse the prelude.
static CSSParserTokenOffsets ReparseForOffsets(
const StringView prelude,
const CSSParserTokenRange range) {
Vector<wtf_size_t, 32> raw_offsets =
CSSTokenizer(prelude).TokenizeToEOFWithOffsets().second;
return {range.RemainingSpan(), std::move(raw_offsets), prelude};
}

StyleRuleImport* CSSParserImpl::ConsumeImportRule(
const AtomicString& uri,
CSSParserTokenStream& stream) {
Expand All @@ -890,6 +899,11 @@ StyleRuleImport* CSSParserImpl::ConsumeImportRule(
return nullptr; // Parse error, expected string or URI
}

CSSParserTokenOffsets offsets = ReparseForOffsets(
stream.StringRangeAt(prelude_offset_start,
prelude_offset_end - prelude_offset_start),
prelude);

StyleRuleBase::LayerName layer;
if (prelude.Peek().GetType() == kIdentToken &&
prelude.Peek().Id() == CSSValueID::kLayer) {
Expand Down Expand Up @@ -921,7 +935,7 @@ StyleRuleImport* CSSParserImpl::ConsumeImportRule(

return MakeGarbageCollected<StyleRuleImport>(
uri, std::move(layer),
MediaQueryParser::ParseMediaQuerySet(prelude,
MediaQueryParser::ParseMediaQuerySet(prelude, offsets,
context_->GetExecutionContext()),
context_->IsOriginClean() ? OriginClean::kTrue : OriginClean::kFalse);
}
Expand Down Expand Up @@ -988,7 +1002,9 @@ StyleRuleMedia* CSSParserImpl::ConsumeMediaRule(
.StringRangeAt(prelude_offset_start,
prelude_offset_end - prelude_offset_start)
.ToString();
const MediaQuerySet* media = CachedMediaQuerySet(prelude_string, prelude);
CSSParserTokenOffsets offsets = ReparseForOffsets(prelude_string, prelude);
const MediaQuerySet* media =
CachedMediaQuerySet(prelude_string, prelude, offsets);
DCHECK(media);

if (RuntimeEnabledFeatures::CSSNestingEnabled() &&
Expand Down Expand Up @@ -1601,6 +1617,11 @@ StyleRuleContainer* CSSParserImpl::ConsumeContainerRule(

ContainerQueryParser query_parser(*context_);

CSSParserTokenOffsets offsets = ReparseForOffsets(
stream.StringRangeAt(prelude_offset_start,
prelude_offset_end - prelude_offset_start),
prelude);

// <container-name>
AtomicString name;
if (prelude.Peek().GetType() == kIdentToken) {
Expand All @@ -1611,7 +1632,8 @@ StyleRuleContainer* CSSParserImpl::ConsumeContainerRule(
}
}

const MediaQueryExpNode* query = query_parser.ParseCondition(prelude);
const MediaQueryExpNode* query =
query_parser.ParseCondition(prelude, offsets);
if (!query) {
return nullptr;
}
Expand Down Expand Up @@ -2313,12 +2335,13 @@ std::unique_ptr<Vector<KeyframeOffset>> CSSParserImpl::ConsumeKeyframeKeyList(

const MediaQuerySet* CSSParserImpl::CachedMediaQuerySet(
String prelude_string,
CSSParserTokenRange prelude) {
CSSParserTokenRange prelude,
const CSSParserTokenOffsets& offsets) {
Member<const MediaQuerySet>& media =
media_query_cache_.insert(prelude_string, nullptr).stored_value->value;
if (!media) {
media = MediaQueryParser::ParseMediaQuerySet(
prelude, context_->GetExecutionContext());
prelude, offsets, context_->GetExecutionContext());
}
DCHECK(media);
return media.Get();
Expand Down

0 comments on commit d1c91c6

Please sign in to comment.