search

LadybirdBrowser / ladybird

Truly independent web browser
https://ladybird.org
BSD 2-Clause "Simplified" License
22.36k stars 994 forks source link

LibWeb: Several fixes for CSS selectors #2478

Closed Gingeh closed 2 days ago

Gingeh commented 4 days ago

Passes 140 new tests, mostly in /dom/nodes/ParentNode-querySelector-All.html.

I ran /css/selectors, /css/css-syntax, and /dom/nodes after each change to make sure nothing regressed.

ladybird-bot commented 4 days ago

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

Gingeh commented 4 days ago

Remaining failures:

Gingeh commented 3 days ago

The included test is too slow to run on CI :( (although it is very fast via Meta/WPT.sh)

Also the GNU CI runner doesn't like how I've used consume_a_list_of_component_values in SelectorParsing.cpp

AtkinsSJ commented 3 days ago

Also the GNU CI runner doesn't like how I've used consume_a_list_of_component_values in SelectorParsing.cpp

You might need to force-instantiate that in Parser.cpp, the parser does some funky stuff with templates because I didn't know any better at the time. 😅

I can't check right now but some methods in that file already have this. After the function body there are a couple of lines redeclaring it using the templated types explicitly which tells the compiler to actually generate those. You'll need to do that. Probably we should have that under every method that takes a generic TokenStream.

Gingeh commented 3 days ago

You might need to force-instantiate that in Parser.cpp After the function body there are a couple of lines redeclaring it using the templated types explicitly which tells the compiler to actually generate those. You'll need to do that.

Ah, thank you! (edit: it worked!)

Probably we should have that under every method that takes a generic TokenStream.

Could I do that as a separate commit before the others in this PR or would that be better as a separate PR?

Gingeh commented 2 days ago

Update: Added a fix for the #id-div1, #id-div1 subtests