Summary: | CSS2/3: Descendant and indirect adjacent selectors don't backtrack properly | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||||||||||||
Component: | CSS | Assignee: | Dave Hyatt <hyatt> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | allan.jensen, bugs-webkit, eric, ian, nickshanks, phiw2 | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 412 | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | OS X 10.4 | ||||||||||||||||
Attachments: |
|
Description
Dave Hyatt
2005-06-10 22:49:40 PDT
Created attachment 2243 [details]
Test showing the problem (works in Firefox)
Created attachment 4059 [details]
Another test case (works with firefox / opera PC)
The same bug is tracked in KDE as http://bugs.kde.org/show_bug.cgi?id=101115 Created attachment 15952 [details]
Quick patch
Here is a quick patch that fixes this bug isolated.
The patch is not ready for review yet, but I will submit it later as part of a larger patch that fixes several serious issues in WebKit's CSS Selector implementation.
Created attachment 15980 [details]
Patch
A more complete patch. It cleans up minor crud in the CSS selector handling, but stays clear of any major changes.
I decided a larger patch probably wasn't suitable at this time.
Comment on attachment 15980 [details]
Patch
forget it, it fails a few regression-tests, now that I got the tests working.
Created attachment 15995 [details]
New patch
Updated patch. I had missed how you handled pseudostyles differently.
I still get regression-failures, but I assume now that the rest are normal for the WebKitQt build.
Comment on attachment 15995 [details]
New patch
This looks ok. What did you mean when you said that there are still failures? Are there tests we need to worry about (or do they just need to be fixed because they pass now)?
I have around 100 test that fails with small stuff like varying heights. I didn't read all of them. I assume that most are just the difference between Mac and Qt versions, but some are likely to be improvements too. I am still working on getting the tests to run properly. The only two failures I get on the Mac are fast/js/function-decompilation-operators.html and http/tests/security/cross-frame-access-protocol-explicit.html. I doubt these are failing due to the patch. Comment on attachment 15995 [details]
New patch
Given Cameron's comment and hyatt's previous comment, r=me for the feature-branch. We may need to roll feature branch forward for this to apply cleanly.
Created attachment 16305 [details]
Updated patch
This updated patch fixes a possible performance regression I thought would never occur in the wild.
The regression occured on a very deep tree with similar nested nodes, and a 3 or more part descent selector that matched these nodes. Unfortunately I found a case with "XHTML served as HTML" this week that did just that, because a lot of elements were never closed.
Just curious what the status here is. Did you mean to mark that patch for review? The old patch was approved. This new patch is just a speed-up. Comment on attachment 16305 [details]
Updated patch
I know mjs already reviewed the old one, but I figured I should at least look before landing. Still looks sane. r=me.
Landed on feature-branch. |