Bug 18027

Created attachment 19987 [details] test case

Created attachment 23996 [details] Mac OS X-like dock tryout with 2 nextsibling selectors (like b:hover + b + b). Added this attachment o show why this bug is important (to me). If you want to create a mac os x-style dock, you need the span:hover+span+span selector to work. I removed all the styling to make the item under the mouse pointer the largest, for easier testing. Move the mouse from a to g to see the effect. The 3rd letter should be bigger.

Ok, first post, I'll prolly do a lot of things wrong here : ) I ran into this bug while building a CSS map, and figured it was a bug and wrote it off. But making more maps showed that *sometimes* webkit DOES work, though not well. I made a simple test case, however I have not been able to discover what makes it *kinda* work-- I've tried to undo every difference between my two pages. I still have a few ideas to try out but it will take a long time. My test page with the reduced bug has links to the two complicated cases, "Europa" where it completely doesn't work, and "North America" where it works partially. Both cases work completely in Konqueror and fail/half-work in Chrome. http://stommepoes.nl/Tests/safaricombinator.html

Created attachment 33521 [details] hover-adjacent-v1 --- 15 files changed, 299 insertions(+), 5 deletions(-)

I've confirmed that overall rendering performance is not degraded by this patch, using iBench 5.0. Here is the result: (1) without the patch HTML Load Speed All iterations 16.49 First iteration (downloaded) 4.6 Subsequent iteration (cached) 1.7 XML / CSS Load Speed All iterations 12.24 First iteration (downloaded) 2.55 Subsequent iteration (cached) 1.94 JavaScript 0.17 HTML W3C DOM Overall 0.34 Sorting 0.25 DOM 0.9 (2) with the patch HTML Load Speed All iterations 15.51 First iteration (downloaded) 3.89 Subsequent iteration (cached) 1.66 XML / CSS Load Speed All iterations 12.19 First iteration (downloaded) 2.57 Subsequent iteration (cached) 1.92 JavaScript 0.15 HTML W3C DOM Overall 0.35 Sorting 0.26 DOM 0.9 --Yusuke (In reply to comment #4) > Created an attachment (id=33521) [details] > hover-adjacent-v1 > > > --- > 15 files changed, 299 insertions(+), 5 deletions(-)

Comment on attachment 33521 [details] hover-adjacent-v1 I'm not sure why your'e using such a super-wide indent her: + function log(msg) { + var console = document.getElementById('console'); + console.innerText += msg; + } Tabs? FAILED/PASSED stuff is done for you if you use the js testing framework in fast/js/resources. Not necessary here, but you should know for the future. It would be better to test these with dumpAsText tests if possible. window.getComputedStyle() should be able to help you there. Hyatt will likely be needed to review the actual code change.

Comment on attachment 33521 [details] hover-adjacent-v1 r- for tabs.

Created attachment 38476 [details] hover-adjacent-v2 --- 9 files changed, 238 insertions(+), 5 deletions(-)

Thanks for your review! I've attached the v2 patch: - added dumpAsText(). - removed PNGs and its checksums. - fixed indent level. Could you take another look?

In general this looks fine to me. The code change itself is about 5 lines. I would like Hyatt to look at the code change and give the final r+. You will have the best luck reaching hyatt via irc or email.

Hyatt, this 3-line patch has been waiting for over a month.

Comment on attachment 38476 [details] hover-adjacent-v2 No objections or commentary in the 3 months that this patch has been up for review. As far as I can tell, this looks fine to me. Yusuke, this will cause failure on other platforms since you're only adding Mac results. We have 3 options: 1. Move the results next to the tests instead of in platform/mac. 2. Add results for other platforms (only necessary if they differ, otherwise #1 is better). 3. Make these tests dumpAsText (possible by using getComputedStyle). If the tests were OK, I would r+ this.

Created attachment 41669 [details] hover-adjacent-v3 --- 9 files changed, 232 insertions(+), 5 deletions(-)

Thanks Eric. Did #1 and #3. Please take another look.

Comment on attachment 41669 [details] hover-adjacent-v3 Sorry. Bugs tend to get lost in the queue. :( OK, tests are better, but still not as easy to read as they should be. + +The green square should turn red when the textarea is focused. + +Test passed. You could easily hide the "green square" stuff when in DumpRenderTree, you can look for layoutTestController. it's OK as is, but could be better. Also: PASS -- explanation why it passed is better than putting "pass" latter in the sentence as it's easier for people to scan. See the script-test examples in the tree, their output is PASS : reason and FAIL : reason. Likewise, this is also confusing when reading as text: +The third square should be green and turn red when the pointer hovers over the first black square. + +Test for https://bugs.webkit.org/show_bug.cgi?id=18027 - CSS3 Selector Test: combination of hover and multiple chained sibling selector fails in Webkit. + +The hover effect works! Extra junk not needed in tests: +<!DOCTYPE html> +<html> +<head> +<title></title> Overall this looks like a good change. I really need to write a document about writing great layout tests. Your tests should be as short as possible and super-easy to read/understand. The problem is that most people won't have nearly the level of understanding that you do about the issue, so you need to make your tests super-easy to read/understand so that they can make quick judgements about what is passing/failing or what might be wrong. This is why we try to encourage things like script-tests as they're generally easy to read. :) Why do the tests need to be run from onload? It's OK for them to be run from onload, but moving the <script> tag to the end of the body instead would have the same effect (or very similar). I think we should go one more round on improving the tests here. Please just email me when you update it and I'll be happy to review it right away! Again, my apologies for the horrible delay!

Created attachment 53654 [details] Test page to reproduce problem. Test file to reproduce bug. Also reproduces Bug 12520 and I attached it there too.

I just want to mention that this applies to hover with general sibling combinator as well. An example here: http://dl.dropbox.com/u/952/cssvocab/index.html Hover on the left curly bracket in Webkit browsers and on non-webkit browsers to see the bug.

Unable to reproduce in current WebKit Nightly.