fast/history/clicked-link-is-visited is failing on the buildbot.
<rdar://problem/5237078>
This does not seem to reproduce outside of DumpRenderTree.
Looks like this was caused by http://trac.webkit.org/projects/webkit/changeset/21649
Working on this.
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.
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.
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.
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.
Created attachment 14926 [details] 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.
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.
Comment on attachment 14926 [details] Fix Adele's test to reload through DRT dear beth, you rock. from, adele
Dear Adele, YOU TOOOO! From, Beth p.s. Fixed with r22087 p.p.s. adeleandbethrock.com