Skip to content

Commit 6e28c2e

Browse files
authored
fix: css important semantics (#792)
- **fix(css): align !important semantics across cascade** - **perf(css): speed up CSSStyleDeclaration union/merge** - **perf(css): skip pseudo-element matching when unused** - **refactor(css): make inline styles Dart-only** - **perf(css): gate pseudo emissions by rule features** - **test(css): update snapshot for relayout-align-to-stretch integration test** - **fix(bridge): stabilize style export traversal** - **perf(bridge): treat kLocalStyleChange as element-only** <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Inline and sheet styles now support !important and explicit sheet-style updates for correct cascade behavior. * Better support for pseudo-element styles (before/after/first-letter/first-line). * **Bug Fixes** * More accurate style merging and fallback between inline and stylesheet rules; computed styles reflect priority changes. * **Chores** * Refactored style handling and improved style recalculation performance (reduced traversal work, caching). * **Tests** * Added and updated unit/integration tests covering important semantics and inline/sheet interactions. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 102057c + 101b12b commit 6e28c2e

File tree

65 files changed

+3012
-1374
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+3012
-1374
lines changed

bridge/bindings/qjs/converter_impl.h

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
#include "core/css/computed_css_style_declaration.h"
3737
#include "core/css/legacy/legacy_computed_css_style_declaration.h"
38+
#include "foundation/utility/make_visitor.h"
3839

3940
namespace webf {
4041

@@ -637,56 +638,6 @@ struct Converter<IDLNullable<T, typename std::enable_if_t<std::is_base_of<Script
637638
}
638639
};
639640

640-
template <>
641-
struct Converter<ElementStyle> {
642-
using ImplType = ElementStyle;
643-
644-
static ElementStyle FromValue(JSContext* ctx, JSValue value, ExceptionState& exception_state) {
645-
auto ectx = ExecutingContext::From(ctx);
646-
if (ectx->isBlinkEnabled()) {
647-
if (JS_IsNull(value)) {
648-
return static_cast<InlineCssStyleDeclaration*>(nullptr);
649-
}
650-
651-
return Converter<InlineCssStyleDeclaration>::FromValue(ctx, value, exception_state);
652-
} else {
653-
if (JS_IsNull(value)) {
654-
return static_cast<legacy::LegacyInlineCssStyleDeclaration*>(nullptr);
655-
}
656-
657-
return Converter<legacy::LegacyInlineCssStyleDeclaration>::FromValue(ctx, value, exception_state);
658-
}
659-
}
660-
661-
static ElementStyle ArgumentsValue(ExecutingContext* context,
662-
JSValue value,
663-
uint32_t argv_index,
664-
ExceptionState& exception_state) {
665-
if (context->isBlinkEnabled()) {
666-
if (JS_IsNull(value)) {
667-
return static_cast<InlineCssStyleDeclaration*>(nullptr);
668-
}
669-
670-
return Converter<InlineCssStyleDeclaration>::ArgumentsValue(context, value, argv_index, exception_state);
671-
} else {
672-
if (JS_IsNull(value)) {
673-
return static_cast<legacy::LegacyInlineCssStyleDeclaration*>(nullptr);
674-
}
675-
676-
return Converter<legacy::LegacyInlineCssStyleDeclaration>::ArgumentsValue(context, value, argv_index, exception_state);
677-
}
678-
}
679-
680-
static JSValue ToValue(JSContext* ctx, ElementStyle value) {
681-
return std::visit(MakeVisitor([&ctx](auto* style) {
682-
if (style == nullptr)
683-
return JS_NULL;
684-
return Converter<std::remove_pointer_t<std::decay_t<decltype(style)>>>::ToValue(ctx, style);
685-
}),
686-
value);
687-
}
688-
};
689-
690641
template <>
691642
struct Converter<WindowComputedStyle> {
692643
using ImplType = WindowComputedStyle;

bridge/core/api/element.cc

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,20 @@
1111
#include "core/css/legacy/legacy_inline_css_style_declaration.h"
1212
#include "core/dom/container_node.h"
1313
#include "core/dom/element.h"
14-
#include "foundation/utility/make_visitor.h"
1514

1615
namespace webf {
1716

1817
WebFValue<LegacyCssStyleDeclaration, LegacyCssStyleDeclarationPublicMethods> ElementPublicMethods::Style(Element* ptr) {
1918
auto* element = static_cast<webf::Element*>(ptr);
2019
MemberMutationScope member_mutation_scope{element->GetExecutingContext()};
21-
auto style = element->style();
20+
auto* style_declaration = element->style();
21+
if (!style_declaration) {
22+
return WebFValue<LegacyCssStyleDeclaration, LegacyCssStyleDeclarationPublicMethods>::Null();
23+
}
2224

23-
return std::visit(
24-
MakeVisitor(
25-
[&](legacy::LegacyInlineCssStyleDeclaration* styleDeclaration) {
26-
WebFValueStatus* status_block = styleDeclaration->KeepAlive();
27-
return WebFValue<LegacyCssStyleDeclaration, LegacyCssStyleDeclarationPublicMethods>(
28-
styleDeclaration, styleDeclaration->legacyCssStyleDeclarationPublicMethods(), status_block);
29-
},
30-
[](auto&&) { return WebFValue<LegacyCssStyleDeclaration, LegacyCssStyleDeclarationPublicMethods>::Null(); }),
31-
style);
25+
WebFValueStatus* status_block = style_declaration->KeepAlive();
26+
return WebFValue<LegacyCssStyleDeclaration, LegacyCssStyleDeclarationPublicMethods>(
27+
style_declaration, style_declaration->legacyCssStyleDeclarationPublicMethods(), status_block);
3228
}
3329

3430
void ElementPublicMethods::ToBlob(Element* ptr,

bridge/core/css/blink_inline_style_validation_test.cc

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ std::string CommandArg01ToUTF8(const UICommandItem& item) {
2626
return String(utf16, static_cast<size_t>(item.args_01_length)).ToUTF8String();
2727
}
2828

29-
std::string SharedNativeStringToUTF8(const SharedNativeString* s) {
29+
std::string SharedNativeStringToUTF8(const webf::SharedNativeString* s) {
3030
if (!s || !s->string() || s->length() == 0) {
3131
return "";
3232
}
@@ -53,7 +53,7 @@ bool HasSetStyleWithKeyValue(ExecutingContext* context, const std::string& key,
5353
auto* items = static_cast<UICommandItem*>(pack->data);
5454
for (int64_t i = 0; i < pack->length; ++i) {
5555
const UICommandItem& item = items[i];
56-
if (item.type == static_cast<int32_t>(UICommand::kSetStyle)) {
56+
if (item.type == static_cast<int32_t>(UICommand::kSetInlineStyle)) {
5757
if (CommandArg01ToUTF8(item) != key) {
5858
continue;
5959
}
@@ -79,34 +79,7 @@ bool HasSetStyleWithKeyValue(ExecutingContext* context, const std::string& key,
7979
if (item.string_01 < 0) {
8080
value_text = getValueName(static_cast<CSSValueID>(-item.string_01 - 1));
8181
} else {
82-
auto* value_ptr = reinterpret_cast<SharedNativeString*>(static_cast<uintptr_t>(item.string_01));
83-
value_text = SharedNativeStringToUTF8(value_ptr);
84-
}
85-
if (value_text == value) {
86-
return true;
87-
}
88-
}
89-
return false;
90-
}
91-
92-
bool HasSetStyleByIdWithKeyValue(ExecutingContext* context, const std::string& key, const std::string& value) {
93-
const CSSPropertyID expected_property_id = CssPropertyID(context, ConvertCamelCaseToKebabCase(key));
94-
auto* pack = static_cast<UICommandBufferPack*>(context->uiCommandBuffer()->data());
95-
auto* items = static_cast<UICommandItem*>(pack->data);
96-
for (int64_t i = 0; i < pack->length; ++i) {
97-
const UICommandItem& item = items[i];
98-
if (item.type != static_cast<int32_t>(UICommand::kSetStyleById)) {
99-
continue;
100-
}
101-
if (item.args_01_length != static_cast<int32_t>(expected_property_id)) {
102-
continue;
103-
}
104-
105-
std::string value_text;
106-
if (item.string_01 < 0) {
107-
value_text = getValueName(static_cast<CSSValueID>(-item.string_01 - 1));
108-
} else {
109-
auto* value_ptr = reinterpret_cast<SharedNativeString*>(static_cast<uintptr_t>(item.string_01));
82+
auto* value_ptr = reinterpret_cast<webf::SharedNativeString*>(static_cast<uintptr_t>(item.string_01));
11083
value_text = SharedNativeStringToUTF8(value_ptr);
11184
}
11285
if (value_text == value) {
@@ -118,7 +91,7 @@ bool HasSetStyleByIdWithKeyValue(ExecutingContext* context, const std::string& k
11891

11992
} // namespace
12093

121-
TEST(BlinkCSSStyleDeclarationValidation, RejectsInvalidFontSize) {
94+
TEST(BlinkCSSStyleDeclarationValidation, ForwardsInvalidFontSizeToDart) {
12295
bool static errorCalled = false;
12396
webf::WebFPage::consoleMessageHandler = [](void*, const std::string&, int) {};
12497

@@ -137,15 +110,15 @@ TEST(BlinkCSSStyleDeclarationValidation, RejectsInvalidFontSize) {
137110
const char* set_valid = "document.body.style.fontSize = '18px';";
138111
env->page()->evaluateScript(set_valid, strlen(set_valid), "vm://", 0);
139112
TEST_runLoop(context);
140-
EXPECT_TRUE(HasSetStyleByIdWithKeyValue(context, "fontSize", "18px"));
113+
EXPECT_TRUE(HasSetStyleWithKeyValue(context, "fontSize", "18px"));
141114

142115
context->uiCommandBuffer()->clear();
143116

144-
// Invalid font-size should be rejected on the native (Blink) CSS side and
145-
// thus should not be forwarded to Dart.
117+
// Inline style is legacy-only even when Blink CSS is enabled; invalid values
118+
// are forwarded to Dart for validation/handling.
146119
const char* set_invalid = "document.body.style.fontSize = '-1px';";
147120
env->page()->evaluateScript(set_invalid, strlen(set_invalid), "vm://", 0);
148121
TEST_runLoop(context);
149-
EXPECT_FALSE(HasSetStyleByIdWithKeyValue(context, "fontSize", "-1px"));
122+
EXPECT_TRUE(HasSetStyleWithKeyValue(context, "fontSize", "-1px"));
150123
EXPECT_EQ(errorCalled, false);
151124
}

bridge/core/css/css_properties.json5

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@
11031103
},
11041104
{
11051105
name: "list-style-type",
1106-
// Parsing and initial value only – forwarded to Dart via UICommand kSetStyle
1106+
// Parsing and initial value only – forwarded to Dart via UICommand kSetInlineStyle
11071107
// We don't currently store this on ComputedStyle; values are emitted from the inline
11081108
// property set during style resolution.
11091109
field_template: "keyword",
@@ -1151,7 +1151,7 @@
11511151
// CSS Counters
11521152
{
11531153
name: "counter-reset",
1154-
// Parsing and initial value only – forwarded to Dart via UICommand kSetStyle
1154+
// Parsing and initial value only – forwarded to Dart via UICommand kSetInlineStyle
11551155
property_methods: [
11561156
"ParseSingleValue",
11571157
"InitialValue"

bridge/core/css/inline_css_style_declaration_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ document.body.style.setProperty('--main-color', 'lightblue'); console.assert(doc
5757

5858
UICommandItem& last = ((UICommandItem*)p_buffer_pack->data)[commandSize - 1];
5959

60-
EXPECT_EQ(last.type, (int32_t)UICommand::kSetStyle);
60+
EXPECT_EQ(last.type, (int32_t)UICommand::kSetInlineStyle);
6161
uint16_t* last_key = (uint16_t*)last.string_01;
6262

6363
// auto native_str = new webf::SharedNativeString(last_key, last.args_01_length);
@@ -101,4 +101,4 @@ TEST(InlineCSSStyleDeclaration, setNullValue) {
101101
"console.assert(document.body.style.height === '')";
102102
env->page()->evaluateScript(code, strlen(code), "vm://", 0);
103103
EXPECT_EQ(errorCalled, false);
104-
}
104+
}

0 commit comments

Comments
 (0)