LibWeb+WebContent: Do not include DOM HTML in text test expectations #1603

trflynn89 · 2024-10-02T16:44:33Z

For example, in the following abbreviated test HTML:

<span>some text</span>
<script>println("whf")</script>

We would have to craft the expectation file to include the "some text"
segment, usually with some leading whitespace. This is a bit annoying,
and makes it difficult to manually craft expectation files.

So instead of comparing the expectation against the entire DOM inner
text, we now send the inner text of just the <pre> element containing
the test output when we invoke internals.signalTextTestIsDone.

circl-lastname · 2024-10-02T16:57:18Z

There are now a couple of tests whose expected files are empty, will a failure still be detected?

Tests/LibWeb/Text/expected/link-element-media-attribute.txt

These tests were mostly async tests written in a manual way. This ports them to use the standard asyncTest() infrastructure. This is mostly just to reduce calls to internals.signalTextTestIsDone, which will have a required parameter in an upcoming test.

The events tested here are decidedly async. We also can't really write sync tests of the form "test(async () => {})". Nothing will await the async callback.

We were signaling that the test is complete too early in some Stream tests.

For example, in the following abbreviated test HTML: <span>some text</span> <script>println("whf")</script> We would have to craft the expectation file to include the "some text" segment, usually with some leading whitespace. This is a bit annoying, and makes it difficult to manually craft expectation files. So instead of comparing the expectation against the entire DOM inner text, we now send the inner text of just the <pre> element containing the test output when we invoke `internals.signalTextTestIsDone`.

awesomekling

Hahaha yes! Yes!!

Ignoring the fact that we should serialize a simplified form of calc() expressions, the following are wrong: - grid-auto-columns - grid-auto-rows - grid-template-columns - grid-template-rows - transform-origin Generated in part with this python script (though I've since iterated on the output repeatedly so it's quite different): ```py import json properties_file = open("./Userland/Libraries/LibWeb/CSS/Properties.json") properties = json.load(properties_file) for (key, value) in properties.items(): if not 'valid-types' in value: continue if 'longhands' in value: continue valid_types = value['valid-types'] for type_string in valid_types: name, *suffix = type_string.split(None, 1) match name: case 'integer' | 'number': print(f'{key}: calc(2 * var(--n));') case 'angle': print(f'{key}: calc(2deg * var(--n));') case 'flex': print(f'{key}: calc(2fr * var(--n));') case 'frequency': print(f'{key}: calc(2hz * var(--n));') case 'length': print(f'{key}: calc(2px * var(--n));') case 'percentage': print(f'{key}: calc(2% * var(--n));') case 'resolution': print(f'{key}: calc(2x * var(--n));') case 'time': print(f'{key}: calc(2s * var(--n));') ``` (cherry picked from commit 301502a3a12b86e218f477f06d2d5271aee8e017; amended to include `Well, hello friends ` in expected/css/calc-coverage.txt because we don't have LadybirdBrowser/ladybird#1603 yet)

(cherry picked from commit 4408ea7c9be1b3faefb5d68871ddd60f912a431b; amended to add a trailing space to calc-missing-context.txt since we don't have LadybirdBrowser/ladybird#1603 yet)

This reverts 6d25bf3 Invalidating the style here means that transitions can cause an element to leave style computation with its "needs style update" flag set to true. This then causes a VERIFY to fail in the TreeBuilder. This invalidation does not otherwise seem to have any effect. The original commit suggests this was to fix a bug, but it's not clear what bug that was. If it reappears, we can try to solve the issue in a different way. (cherry picked from commit 81596b41457d250bac28db37c1390fa46e6532de; amended test with leading whitespace since we don't have LadybirdBrowser/ladybird#1603 yet)

Before this change we were serializing them in a bogus 8-digit hex color format that isn't actually recognized by HTML. This code will need more work when we start supporting color spaces other than sRGB. (cherry picked from commit 4590c081c2eb257232463ed660498a02f9bbe7b8; amended expected output because serenity does not yet have LadybirdBrowser/ladybird#1603)

This matches the behavior of other browsers and fixes a WPT test. (cherry picked from commit 902586a21de3ef8335e447053d82057c4c927d72; amended expected output because serenity does not yet have LadybirdBrowser/ladybird#1603)

Before this change we were serializing them in a bogus 8-digit hex color format that isn't actually recognized by HTML. This code will need more work when we start supporting color spaces other than sRGB. (cherry picked from commit 4590c081c2eb257232463ed660498a02f9bbe7b8; amended expected output because serenity does not yet have LadybirdBrowser/ladybird#1603)

This matches the behavior of other browsers and fixes a WPT test. (cherry picked from commit 902586a21de3ef8335e447053d82057c4c927d72; amended expected output because serenity does not yet have LadybirdBrowser/ladybird#1603)

CSS Syntax 3 (https://drafts.csswg.org/css-syntax) has changed significantly since we implemented it a couple of years ago. Just about every parsing algorithm has been rewritten in terms of the new token stream concept, and to support nested styles. As all of those algorithms call into each other, this is an unfortunately chonky diff. As part of this, the transitory types (Declaration, Function, AtRule...) have been rewritten. That's both because we have new requirements of what they should be and contain, and also because the spec asks us to create and then gradually modify them in place, which is easier if they are plain structs. (cherry picked from commit e0be17e4fbf1870f35614d0cde8f63e72f78bd16; amended to tweak test expectation due to serenity not yet having LadybirdBrowser/ladybird#1603)

This is because toggling the checkbox is committing the value. (cherry picked from commit 3856dd946b94b61be2ddac80c8cf60278fcc56ab; amended to add two spaces to expected output due to serenity not yet having LadybirdBrowser/ladybird#1603)

This is because toggling the checkbox is committing the value. (cherry picked from commit 3856dd946b94b61be2ddac80c8cf60278fcc56ab; amended to add two spaces to expected output due to serenity not yet having LadybirdBrowser/ladybird#1603, and to add a trailing newline to the test input file)

This is because toggling the checkbox is committing the value. (cherry picked from commit 3856dd946b94b61be2ddac80c8cf60278fcc56ab; amended to add spaces to expected output due to serenity not yet having LadybirdBrowser/ladybird#1603, and to add a trailing newline to the test input file)

trflynn89 force-pushed the text_tests branch 2 times, most recently from cdd45f9 to ddd104a Compare October 2, 2024 16:51

trflynn89 force-pushed the text_tests branch from ddd104a to bccfa46 Compare October 2, 2024 16:58

circl-lastname reviewed Oct 2, 2024

View reviewed changes

Tests/LibWeb/Text/expected/link-element-media-attribute.txt Outdated Show resolved Hide resolved

trflynn89 force-pushed the text_tests branch from bccfa46 to c60d408 Compare October 2, 2024 17:43

trflynn89 force-pushed the text_tests branch 2 times, most recently from 21f237f to 922760e Compare October 2, 2024 18:03

trflynn89 added 3 commits October 2, 2024 14:06

LibWeb: Convert some sync tests to be async

9ed600a

The events tested here are decidedly async. We also can't really write sync tests of the form "test(async () => {})". Nothing will await the async callback.

LibWeb: Wait for the correct condition in Stream tests

a5ecaf5

We were signaling that the test is complete too early in some Stream tests.

trflynn89 force-pushed the text_tests branch from 922760e to 5cc35fe Compare October 2, 2024 18:06

awesomekling approved these changes Oct 3, 2024

View reviewed changes

kalenikaliaksandr approved these changes Oct 3, 2024

View reviewed changes

trflynn89 merged commit bf66869 into LadybirdBrowser:master Oct 3, 2024
6 checks passed

trflynn89 deleted the text_tests branch October 3, 2024 11:07

trflynn89 mentioned this pull request Oct 3, 2024

headless-browser: Add a flag to rebaseline executed layout / text tests #1612

Merged

nico mentioned this pull request Nov 9, 2024

Cherry-pick "LibWeb: Handle calculations without a context better" SerenityOS/serenity#25286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LibWeb+WebContent: Do not include DOM HTML in text test expectations #1603

LibWeb+WebContent: Do not include DOM HTML in text test expectations #1603

trflynn89 commented Oct 2, 2024

circl-lastname commented Oct 2, 2024

awesomekling left a comment

LibWeb+WebContent: Do not include DOM HTML in text test expectations #1603

LibWeb+WebContent: Do not include DOM HTML in text test expectations #1603

Conversation

trflynn89 commented Oct 2, 2024

circl-lastname commented Oct 2, 2024

awesomekling left a comment

Choose a reason for hiding this comment