Fast/history/back-forward-reset-after-error-handling.html calls handleErrorPages(), which was not supported by DumpRenderTree and is not supported by TestRunner. Given that this test was added for QT support, it should be removed.
Created attachment 285331 [details]
(In reply to comment #0)
> Fast/history/back-forward-reset-after-error-handling.html calls
> handleErrorPages(), which was not supported by DumpRenderTree and is not
> supported by TestRunner. Given that this test was added for QT support, it
> should be removed.
This description is disingenuous. This test LayoutTests/fast/history/back-forward-reset-after-error-handling.html was added for all ports that implemented testRunner.handleErrorPages(). As it turns out testRunner.handleErrorPages() was only implemented on the Qt WebKit port.
For completeness, the motivation for adding testRunner.handleErrorPages() is described in bug #31509, comment 0. And the bug #31555 was filed to implement this feature for Mac though it was resolved as WONTFIX on 05/01/2012.
Comment on attachment 285331 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=285331&action=review
The motivation for this change seems reasonable. We need to also update file LayoutTests/platform/ios-simulator/TestExpectations. Also we should update the ChangeLog entry such that it list the files modified in this patch as well as clarify the description as per my remarks above.
> + Removed fast/history/back-forward-reset-after-error-handling.html and all
> + references since handleErrorPages() was never supported in TestRunners
How are you generating this ChangeLog entry? Please use prepare-ChangeLog to generate the ChangeLog message.
It is disingenuous to write "was never supported in TestRunners". We should explain that window.testRunner.handleErrorPages() was only implement for the Qt WebKit port and that there has not been interest to implement this feature in any other port in the over 7 years since this feature was added.
Created attachment 285431 [details]
I think the format/comments on this patch should be better.
Comment on attachment 285431 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=285431&action=review
> + handleErrorPages() is not supported by TestRunner, remove dependent test
Please update the title in the ChangeLog to match the Bugzilla title.
> + * platform/gtk/TestExpectations: Dito.
Dito => Ditto.
> + * platform/ios-simulator/TestExpectations: Dito.
> + * platform/mac/TestExpectations: Dito.
> + * platform/win/TestExpectations: Dito.
Created attachment 285440 [details]
Comment on attachment 285440 [details]
Clearing flags on attachment: 285440
Committed r204191: <http://trac.webkit.org/changeset/204191>
All reviewed patches have been landed. Closing bug.