It would be very useful for DumpRenderTree to remember URLs that it has accessed during each run, with the history being empty at the beginning of each run. This would allow for testing history where visited links are a different color than unvisited links. Note that there will be a number of tests whose results will change with this update.
Created attachment 8583 [details] Patch v1 Patch to implement URL history in DumpRenderTree. I still need to review the changed test results to make sure the changes aren't regressions.
Here is a list of tests that will require text and image updates if this patch (Attachment 8583 [details]) lands: fast/block/margin-collapse/010 fast/block/margin-collapse/011 fast/block/margin-collapse/012 fast/block/margin-collapse/015 fast/block/margin-collapse/016 fast/block/margin-collapse/020 fast/block/margin-collapse/056 fast/block/margin-collapse/059 fast/repaint/outline-repaint-glitch tables/mozilla/marvin/backgr_layers-opacity tables/mozilla/marvin/backgr_position-table tables/mozilla/marvin/backgr_simple-table-cell tables/mozilla/marvin/backgr_simple-table-column tables/mozilla/marvin/backgr_simple-table-column-group tables/mozilla/marvin/backgr_simple-table tables/mozilla/marvin/backgr_simple-table-row tables/mozilla/marvin/backgr_simple-table-row-group tables/mozilla_expected_failures/marvin/backgr_border-table tables/mozilla_expected_failures/marvin/backgr_border-table-column tables/mozilla_expected_failures/marvin/backgr_border-table-row tables/mozilla_expected_failures/marvin/backgr_border-table-row-group tables/mozilla_expected_failures/marvin/backgr_layers-hide tables/mozilla_expected_failures/marvin/backgr_layers-show tables/mozilla_expected_failures/marvin/backgr_position-table-cell tables/mozilla_expected_failures/marvin/backgr_position-table-column tables/mozilla_expected_failures/marvin/backgr_position-table-row tables/mozilla_expected_failures/marvin/backgr_position-table-row-group
Created attachment 8589 [details] Patch v2 Same as Patch v1 with changelogs and *-expected.txt updates. Note that the matching pixel tests will also need to be updated before checking this in. Also note that the current ToT has table layout failures. These should be fixed before applying this patch.
(In reply to comment #3) > Created an attachment (id=8589) [edit] > Also note that the current ToT has table layout failures. These should be > fixed before applying this patch. The expected results for those tests need to be updated. I have attached a patch to bug 8848.
Comment on attachment 8589 [details] Patch v2 What does this mean for running these tests individually?
(In reply to comment #5) > (From update of attachment 8589 [details] [edit]) > What does this mean for running these tests individually? It would mean that an individual test would fail since the color would appear as not-visited instead of visited. All of the current tests whose results would change if the DRT patch were applied are referencing other tests that ran before them when the whole suite is run. There are a number of easy fixes for this: - Change the links to text-only. - Force the link color to stay the same whether visited or not. - Remove the links. Thoughts?
(In reply to comment #6) > (In reply to comment #5) > > What does this mean for running these tests individually? > > It would mean that an individual test would fail since the color would appear > as not-visited instead of visited. The original concept for layout tests was that each test can run separately and give the same results. That means that DumpRenderTree should isolate each test from the next one. A simple way to do that would be to create a new history object each time, as long as that doesn't make the tests too long. But that doesn't allow testing history and in fact it directly contradicts the argument made at the top of this bug report. It seems great for tests that intend to test history to behave this way, but I'd prefer to not have tests affect each other by default. So we should rack our brains and figure out a way to solve the problem.
How about we add a method to the layout test controller which creates a WebHistory object and sets it as the global history. Then we run the test and destroy the WebHistory object afterwards. This way we only create history objects when they are needed and we get isolated tests.
(In reply to comment #8) > How about we add a method to the layout test controller which creates a > WebHistory object and sets it as the global history. Then we run the test and > destroy the WebHistory object afterwards. > > This way we only create history objects when they are needed and we get > isolated tests. Something along those lines sounds excellent; just what we should do. But I still don't understand how tests would work. Are there pairs of tests that go together, or would they be self-contained? I don't need someone to explain it; instead one could just make an example that demonstrates how it will work.
(In reply to comment #9) > But I still don't understand how tests would work. Are there pairs of tests > that go together, or would they be self-contained? > > I don't need someone to explain it; instead one could just make an example that > demonstrates how it will work. See example tests (which are patches that place the tests in the LayoutTests directory structure) in bug 9145, bug 9148 or bug 9166.
Comment on attachment 8589 [details] Patch v2 I'm going to mark this review-, because I'd like to see a version where tests "opt in" to this feature, so that tests can be run singly as well as in groups.
Reassigning this bug back to webkit-unassigned if anyone is interested in fixing this. I plan to come back to it in the next week or two if no one else is working on it.
Created attachment 8892 [details] Patch v3 This patch adds a layoutTestController.keepWebHistory() method to enable web history. It also contains a layout test that demonstrates its usage. The change to the WebCore manual test is to (a) make it work if the JavaScript is commented back in and (b) document the window.history hack it uses. Note that this manual test doesn't exhibit the problem seen in Safari when run by DumpRenderTree (Bug 8079), hence it's a manual test.
Note to self: Update the wiki page when this lands. http://wiki.opendarwin.org/index.php/WebKit:Writing_Layout_Tests_for_DumpRenderTree
Created attachment 8893 [details] Patch v4 Same as Patch v4, except I removed the text/html mime type from the HTML files (svn-apply doesn't handle the property information--yet), and I only call [WebHistory setOptionalSharedHistory:nil] if the field is not currently nil.
Comment on attachment 8893 [details] Patch v4 Looks good, and seems to address Darin's comments. r=me In the layout test, using a fake mouse event to click the link seems a little funny. I'm pretty sure that dispatching an event through the DOM would work just as well. Just for future reference. Part of me also thinks that the repaint and history code should go into separate "controller" objects, not layoutTestController, which should limit itself to direct control of the test mechanism.
Committed revision 14905.
(In reply to comment #14) > Note to self: Update the wiki page when this lands. > http://wiki.opendarwin.org/index.php/WebKit:Writing_Layout_Tests_for_DumpRenderTree Updated.
Created attachment 8909 [details] Fix tests to use DOM mouse events
(In reply to comment #19) > Created an attachment (id=8909) [edit] > Fix tests to use DOM mouse events r=ggaren via IRC Committed revision 14907.