Skip to content

Commit

Permalink
LibWeb/CSS: Make CSS Parser::create() infallible
Browse files Browse the repository at this point in the history 
Now that `Tokenizer::tokenize()` just returns a String, there are no
errors to propagate, and we can simplify the user code a bit.
  • Loading branch information
@AtkinsSJ
AtkinsSJ committed Jul 26, 2024
1 parent b118a75 commit ba7f606
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 64 deletions.
6 changes: 2 additions & 4 deletions Userland/Libraries/LibWeb/Animations/AnimationEffect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,11 +594,9 @@ Optional<double> AnimationEffect::transformed_progress() const

RefPtr<CSS::StyleValue const> AnimationEffect::parse_easing_string(JS::Realm& realm, StringView value)
{
auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);
if (maybe_parser.is_error())
return {};
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);

if (auto style_value = maybe_parser.release_value().parse_as_css_value(CSS::PropertyID::AnimationTimingFunction)) {
if (auto style_value = parser.parse_as_css_value(CSS::PropertyID::AnimationTimingFunction)) {
if (style_value->is_easing())
return style_value;
}
Expand Down
6 changes: 2 additions & 4 deletions Userland/Libraries/LibWeb/Animations/KeyframeEffect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,11 +540,9 @@ static WebIDL::ExceptionOr<Vector<BaseKeyframe>> process_a_keyframes_argument(JS
if (!property_id.has_value())
continue;

auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value_string);
if (maybe_parser.is_error())
continue;
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value_string);

if (auto style_value = maybe_parser.release_value().parse_as_css_value(*property_id)) {
if (auto style_value = parser.parse_as_css_value(*property_id)) {
// Handle 'initial' here so we don't have to get the default value of the property every frame in StyleComputer
if (style_value->is_initial())
style_value = CSS::property_initial_value(realm, *property_id);
Expand Down
18 changes: 5 additions & 13 deletions Userland/Libraries/LibWeb/CSS/FontFace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,8 @@ JS_DEFINE_ALLOCATOR(FontFace);
template<CSS::PropertyID PropertyID>
RefPtr<CSS::StyleValue const> parse_property_string(JS::Realm& realm, StringView value)
{
auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);
if (maybe_parser.is_error())
return {};

return maybe_parser.release_value().parse_as_css_value(PropertyID);
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);
return parser.parse_as_css_value(PropertyID);
}

// https://drafts.csswg.org/css-font-loading/#font-face-constructor
Expand All @@ -92,15 +89,10 @@ JS::NonnullGCPtr<FontFace> FontFace::construct_impl(JS::Realm& realm, String fam
Vector<CSS::ParsedFontFace::Source> sources;
ByteBuffer buffer;
if (auto* string = source.get_pointer<String>()) {
auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm, base_url), *string);
if (maybe_parser.is_error()) {
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm, base_url), *string);
sources = parser.parse_as_font_face_src();
if (sources.is_empty())
WebIDL::reject_promise(realm, promise, WebIDL::SyntaxError::create(realm, "FontFace constructor: Invalid source string"_fly_string));
} else {
auto parser = maybe_parser.release_value();
sources = parser.parse_as_font_face_src();
if (sources.is_empty())
WebIDL::reject_promise(realm, promise, WebIDL::SyntaxError::create(realm, "FontFace constructor: Invalid source string"_fly_string));
}
} else {
auto buffer_source = source.get<JS::Handle<WebIDL::BufferSource>>();
auto maybe_buffer = WebIDL::get_buffer_source_copy(buffer_source->raw_object());
Expand Down
27 changes: 9 additions & 18 deletions Userland/Libraries/LibWeb/CSS/Parser/Helpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,64 +22,55 @@ CSS::CSSStyleSheet* parse_css_stylesheet(CSS::Parser::ParsingContext const& cont
auto media_list = CSS::MediaList::create(context.realm(), {});
return CSS::CSSStyleSheet::create(context.realm(), rule_list, media_list, location);
}
auto parser = CSS::Parser::Parser::create(context, css).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_css_stylesheet(location);
return CSS::Parser::Parser::create(context, css).parse_as_css_stylesheet(location);
}

CSS::ElementInlineCSSStyleDeclaration* parse_css_style_attribute(CSS::Parser::ParsingContext const& context, StringView css, DOM::Element& element)
{
if (css.is_empty())
return CSS::ElementInlineCSSStyleDeclaration::create(element, {}, {});
auto parser = CSS::Parser::Parser::create(context, css).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_style_attribute(element);
return CSS::Parser::Parser::create(context, css).parse_as_style_attribute(element);
}

RefPtr<CSS::StyleValue> parse_css_value(CSS::Parser::ParsingContext const& context, StringView string, CSS::PropertyID property_id)
{
if (string.is_empty())
return nullptr;
auto parser = MUST(CSS::Parser::Parser::create(context, string));
return parser.parse_as_css_value(property_id);
return CSS::Parser::Parser::create(context, string).parse_as_css_value(property_id);
}

CSS::CSSRule* parse_css_rule(CSS::Parser::ParsingContext const& context, StringView css_text)
{
auto parser = CSS::Parser::Parser::create(context, css_text).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_css_rule();
return CSS::Parser::Parser::create(context, css_text).parse_as_css_rule();
}

Optional<CSS::SelectorList> parse_selector(CSS::Parser::ParsingContext const& context, StringView selector_text)
{
auto parser = CSS::Parser::Parser::create(context, selector_text).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_selector();
return CSS::Parser::Parser::create(context, selector_text).parse_as_selector();
}

RefPtr<CSS::MediaQuery> parse_media_query(CSS::Parser::ParsingContext const& context, StringView string)
{
auto parser = CSS::Parser::Parser::create(context, string).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_media_query();
return CSS::Parser::Parser::create(context, string).parse_as_media_query();
}

Vector<NonnullRefPtr<CSS::MediaQuery>> parse_media_query_list(CSS::Parser::ParsingContext const& context, StringView string)
{
auto parser = CSS::Parser::Parser::create(context, string).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_media_query_list();
return CSS::Parser::Parser::create(context, string).parse_as_media_query_list();
}

RefPtr<CSS::Supports> parse_css_supports(CSS::Parser::ParsingContext const& context, StringView string)
{
if (string.is_empty())
return {};
auto parser = CSS::Parser::Parser::create(context, string).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_supports();
return CSS::Parser::Parser::create(context, string).parse_as_supports();
}

Optional<CSS::StyleProperty> parse_css_supports_condition(CSS::Parser::ParsingContext const& context, StringView string)
{
if (string.is_empty())
return {};
auto parser = CSS::Parser::Parser::create(context, string).release_value_but_fixme_should_propagate_errors();
return parser.parse_as_supports_condition();
return CSS::Parser::Parser::create(context, string).parse_as_supports_condition();
}

}
22 changes: 11 additions & 11 deletions Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static void log_parse_error(SourceLocation const& location = SourceLocation::cur

namespace Web::CSS::Parser {

ErrorOr<Parser> Parser::create(ParsingContext const& context, StringView input, StringView encoding)
Parser Parser::create(ParsingContext const& context, StringView input, StringView encoding)
{
auto tokens = Tokenizer::tokenize(input, encoding);
return Parser { context, move(tokens) };
Expand Down Expand Up @@ -7788,7 +7788,7 @@ NonnullRefPtr<StyleValue> Parser::resolve_unresolved_style_value(ParsingContext

// If the value is invalid, we fall back to `unset`: https://www.w3.org/TR/css-variables-1/#invalid-at-computed-value-time

auto parser = MUST(Parser::create(context, ""sv));
auto parser = Parser::create(context, ""sv);
return parser.resolve_unresolved_style_value(element, pseudo_element, property_id, unresolved);
}

Expand Down Expand Up @@ -8072,7 +8072,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
auto attribute_value = element.get_attribute_value(attribute_name);
if (attribute_type.equals_ignoring_ascii_case("angle"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <dimension-token> whose unit matches the given type, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Dimension)) {
Expand All @@ -8085,7 +8085,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
// Parse a component value from the attribute’s value.
// If the result is a <hex-color> or a named color ident, the substitution value is that result as a <color>.
// Otherwise there is no substitution value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
if (component_value.has_value()) {
if ((component_value->is(Token::Type::Hash)
&& Color::from_string(MUST(String::formatted("#{}", component_value->token().hash_value()))).has_value())
Expand All @@ -8097,7 +8097,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
}
} else if (attribute_type.equals_ignoring_ascii_case("flex"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <dimension-token> whose unit matches the given type, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Dimension)) {
Expand All @@ -8108,7 +8108,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
}
} else if (attribute_type.equals_ignoring_ascii_case("frequency"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <dimension-token> whose unit matches the given type, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Dimension)) {
Expand All @@ -8131,7 +8131,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
}
} else if (attribute_type.equals_ignoring_ascii_case("length"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <dimension-token> whose unit matches the given type, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Dimension)) {
Expand All @@ -8144,14 +8144,14 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
// Parse a component value from the attribute’s value.
// If the result is a <number-token>, the result is the substitution value.
// Otherwise, there is no substitution value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
if (component_value.has_value() && component_value->is(Token::Type::Number)) {
dest.append(component_value.release_value());
return true;
}
} else if (attribute_type.equals_ignoring_ascii_case("percentage"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <percentage-token>, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Percentage)) {
Expand All @@ -8166,7 +8166,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
return true;
} else if (attribute_type.equals_ignoring_ascii_case("time"_fly_string)) {
// Parse a component value from the attribute’s value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
// If the result is a <dimension-token> whose unit matches the given type, the result is the substitution value.
// Otherwise, there is no substitution value.
if (component_value.has_value() && component_value->is(Token::Type::Dimension)) {
Expand All @@ -8186,7 +8186,7 @@ bool Parser::substitute_attr_function(DOM::Element& element, FlyString const& pr
// Parse a component value from the attribute’s value.
// If the result is a <number-token>, the substitution value is a dimension with the result’s value, and the given unit.
// Otherwise, there is no substitution value.
auto component_value = MUST(Parser::Parser::create(m_context, attribute_value)).parse_as_component_value();
auto component_value = Parser::Parser::create(m_context, attribute_value).parse_as_component_value();
if (component_value.has_value() && component_value->is(Token::Type::Number)) {
if (attribute_value == "%"sv) {
dest.empend(Token::create_dimension(component_value->token().number_value(), attribute_type));
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/CSS/Parser/Parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class PropertyDependencyNode;

class Parser {
public:
static ErrorOr<Parser> create(ParsingContext const&, StringView input, StringView encoding = "utf-8"sv);
static Parser create(ParsingContext const&, StringView input, StringView encoding = "utf-8"sv);

Parser(Parser&&);

Expand Down
14 changes: 2 additions & 12 deletions Userland/Libraries/LibWeb/HTML/Canvas/CanvasFillStrokeStyles.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,7 @@ class CanvasFillStrokeStyles {
// 1. If the given value is a string, then:
[&](String const& string) {
// 1. Let context be this's canvas attribute's value, if that is an element; otherwise null.
auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);
if (maybe_parser.is_error()) {
dbgln_if(CANVAS_RENDERING_CONTEXT_2D_DEBUG, "CanvasFillStrokeStyles: Failed to create CSS parser.");
return;
}
auto parser = maybe_parser.release_value();
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);

// 2. Let parsedValue be the result of parsing the given value with context if non-null.
// FIXME: Parse a color value
Expand Down Expand Up @@ -80,12 +75,7 @@ class CanvasFillStrokeStyles {
// 1. If the given value is a string, then:
[&](String const& string) {
// 1. Let context be this's canvas attribute's value, if that is an element; otherwise null.
auto maybe_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);
if (maybe_parser.is_error()) {
dbgln_if(CANVAS_RENDERING_CONTEXT_2D_DEBUG, "CanvasFillStrokeStyles: Failed to create CSS parser.");
return;
}
auto parser = maybe_parser.release_value();
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);

// 2. Let parsedValue be the result of parsing the given value with context if non-null.
// FIXME: Parse a color value
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/HTML/SourceSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ SourceSet parse_a_srcset_attribute(StringView input)
// https://html.spec.whatwg.org/multipage/images.html#parse-a-sizes-attribute
CSS::LengthOrCalculated parse_a_sizes_attribute(DOM::Document const& document, StringView sizes)
{
auto css_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext { document }, sizes).release_value_but_fixme_should_propagate_errors();
auto css_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext { document }, sizes);
return css_parser.parse_as_sizes_attribute();
}

Expand Down

0 comments on commit ba7f606

Please sign in to comment.