13933 2007-05-30 11:00:23 -0700 REGRESSION: fast/history/clicked-link-is-visited is failing 2007-06-10 15:30:14 -0700 1 1 1 Unclassified WebKit History 523.x (Safari 3) Mac OS X 10.4 RESOLVED FIXED http://build.webkit.org/results/post-commit-powerpc-mac-os-x/6871/fast/history/clicked-link-is-visited-diffs.txt InRadar, LayoutTestFailure, Regression P1 Normal --- 1 aroben bdakin adele bdakin oldest_to_newest 8733 0 aroben 2007-05-30 11:00:23 -0700 fast/history/clicked-link-is-visited is failing on the buildbot. 8738 1 aroben 2007-05-30 11:02:11 -0700 <rdar://problem/5237078> 8095 2 bdakin 2007-06-08 15:15:09 -0700 This does not seem to reproduce outside of DumpRenderTree. 8093 3 bdakin 2007-06-08 16:17:52 -0700 Looks like this was caused by http://trac.webkit.org/projects/webkit/changeset/21649 8086 4 bdakin 2007-06-08 16:54:27 -0700 Working on this. 8087 5 bdakin 2007-06-08 19:09:37 -0700 This is difficult to debug because it only happens within DRT and only when you run the entire fast directory, but I have re-arranged the code a bit to get some clues, and I think that this is failing because of the reliance on having a page in the EventHandler::focusDocumentView(), which does seems a bit hard to believe, but it seems like that is the problem. 8088 6 bdakin 2007-06-08 20:33:59 -0700 Actually, that last assessment was incorrect, but what is actually going on is even crazier. This failure has nothing to do with Adele's code change. Instead, it has to do with the test that she added! Deleting that test makes this test pass every time! One I discovered this, I tried to run the two tests back-to-back to see if the failure would reproduce, and it would not. After a lot of testing and reducing, I cam up with a reduced case, which is to run the following three tests in DRT in order: fast/frames/empty.html fast/frames/frame-display-none-focus.html (This is the test Adele added) fast/history/clicked-link-is-visited.html fast/frames/empty.html is not a real test. And it really is empty. For some reason, though, some test, any test!, needs to run before Adele's test to see the bug. So, with THIS reduction, I discovered that the problematic part of Adele's test is: if (parent.location.href.indexOf('?') == -1) { parent.location.href= parent.location.href + "?"; } Removing that part of the test, also makes this other test succeed. I have NO idea how adding a "?" to the parent href would end up affecting history in a later test. I have not figured this out yet at all. But I am also curious about why that is needed for the test anyway. I may test if removing those lines will still make the test a valid layout test for the original issue, but I doubt that Adele would have left it in if it were not necessary. 8066 7 adele 2007-06-08 21:14:25 -0700 The whole '?' thing is to avoid getting in an infinite loop of reloading the page. You need to change the href to trigger a reload, but you only want to do it once. 7996 8 bdakin 2007-06-10 12:33:46 -0700 I re-wrote the test to reload in a different way, and it still caused the other test to fail. So there is something about reloading this page that causes the history/link-coloring test to fail. 7994 9 14926 bdakin 2007-06-10 15:06:52 -0700 Created attachment 14926 Fix Adele's test to reload through DRT I have a fix! Having the test reload through DRT's queueReload mechanism fixes the problem. I confirmed that this is still a valid test for the bug it originally fixed by rolling Adele's patch out of my tree. And sure enough, with Adele's patch gone, this new test also crashes. (Meaning it's valid!) My totally-pulled-out-of-my-ass threory is that reloading through JS got DRT's history all messed up. Maybe off-by-one? So DRT did not color the link the visited color because it did not make it into the history list yet. Or something. 7993 10 bdakin 2007-06-10 15:08:13 -0700 By the way, we should probably fix this problem in DRT. But right now, it is more important to have a green bot. And this is the fastest path to a green bot. 7988 11 14926 adele 2007-06-10 15:20:55 -0700 Comment on attachment 14926 Fix Adele's test to reload through DRT dear beth, you rock. from, adele 7989 12 bdakin 2007-06-10 15:30:14 -0700 Dear Adele, YOU TOOOO! From, Beth p.s. Fixed with r22087 p.p.s. adeleandbethrock.com 14926 2007-06-10 15:06:52 -0700 2007-06-10 15:20:55 -0700 Fix Adele's test to reload through DRT lastLT.txt text/plain 1416 bdakin SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDIyMDc2 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDctMDYt MTAgIGRldGhiYWtpbiAgPGJkYWtpbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4IGZvciBodHRwOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMzkzMyBSRUdSRVNTSU9OOiAKKyAgICAgICAgZmFzdC9oaXN0b3J5 L2NsaWNrZWQtbGluay1pcy12aXNpdGVkIGlzIGZhaWxpbmcKKworICAgICAgICAqIGZhc3QvZnJh bWVzL3Jlc291cmNlcy9pZnJhbWUtZm9jdXMuaHRtbDogQ2F1c2UgdGhlIHBhZ2UgdG8gcmVsb2Fk IAorICAgICAgICB3aXRoIGxheW91dFRlc3RDb250cm9sbGVyLnF1ZXVlUmVsb2FkIGluc3RlYWQg b2YgdXNpbmcgSmF2YVNjcmlwdCAKKyAgICAgICAgdG8gcmVsb2FkLiBVc2luZyBKUyBzZWVtcyB0 byBnZXQgRFJUJ3MgaGlzdG9yeSBhbGwgbWVzc2VkIHVwIChvZmYgCisgICAgICAgIGJ5IG9uZT8h KS4KKwogMjAwNy0wNi0wOCAgZGV0aGJha2luICA8YmRha2luQGFwcGxlLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBCcmFkeS4KSW5kZXg6IGZhc3QvZnJhbWVzL3Jlc291cmNlcy9pZnJhbWUt Zm9jdXMuaHRtbAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBmYXN0L2ZyYW1lcy9yZXNvdXJjZXMvaWZyYW1lLWZv Y3VzLmh0bWwJKHJldmlzaW9uIDIyMDc2KQorKysgZmFzdC9mcmFtZXMvcmVzb3VyY2VzL2lmcmFt ZS1mb2N1cy5odG1sCSh3b3JraW5nIGNvcHkpCkBAIC00LDkgKzQsMTEgQEAKICAgICAgICAgICAg IGZ1bmN0aW9uIHRlc3QoKQogICAgICAgICAgICAgewogICAgICAgICAgICAgICAgIHdpbmRvdy5m b2N1cygpOwotICAgICAgICAgICAgICAgIGlmIChwYXJlbnQubG9jYXRpb24uaHJlZi5pbmRleE9m KCc/JykgPT0gLTEpIHsKLSAgICAgICAgICAgICAgICAgICAgcGFyZW50LmxvY2F0aW9uLmhyZWY9 IHBhcmVudC5sb2NhdGlvbi5ocmVmICsgIj8iOwotICAgICAgICAgICAgICAgIH0KKyAgICAgICAg ICAgICAgICBpZiAoIWxheW91dFRlc3RDb250cm9sbGVyLmJvbykKKyAgICAgICAgICAgICAgICAg ICAgcmV0dXJuOworCisgICAgICAgICAgICAgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIuYm9vID0g ZmFsc2U7CisgICAgICAgICAgICAgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIucXVldWVSZWxvYWQ7 CiAgICAgICAgICAgICB9CiAgICAgICAgIDwvc2NyaXB0PgogICAgIDwvaGVhZD4K