Calling loadAlternateHTMLString on the page proxy repeatedly, or an initial loadAlternateHTMLString and then reload(), will add multiple entries for the substitute-data in the back-forward history, which does not match the behavior of a successful load and subsequent reload. Reproducible in Safari/WebKit nightly by going to http://www.foobarbaz.com/ and clicking reload multiple times.
After checking Safari 5 on 10.5.8, and it also creates a new bf-item for each reload, so perhaps this is intended behavior?
Created attachment 154154 [details] Patch
Created attachment 154171 [details] Patch The old patch was applied to an out of date version and therefore didn't apply cleanly to ToT. This is fixed now.
Attachment 154171 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/loader/FrameLoader.h:106: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 154171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154171&action=review Please fix the ChangeLog and Stylebot's complaint before landing. > Source/WebCore/ChangeLog:13 > + > + Added a TestWebKitAPI test. > + I think we normally mention which API test was added by filename.
(In reply to comment #5) > (From update of attachment 154171 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154171&action=review > > Please fix the ChangeLog and Stylebot's complaint before landing. > > > Source/WebCore/ChangeLog:13 > > + > > + Added a TestWebKitAPI test. > > + > > I think we normally mention which API test was added by filename. I tried to figure out what was making the style bot complain about that line and couldn't figure it out. I also looked for other examples in the changelog for API tests, but couldn't find any. I can give it another look, though.
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 154171 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=154171&action=review > > > > Please fix the ChangeLog and Stylebot's complaint before landing. > > > > > Source/WebCore/ChangeLog:13 > > > + > > > + Added a TestWebKitAPI test. > > > + > > > > I think we normally mention which API test was added by filename. > > I tried to figure out what was making the style bot complain about that line and couldn't figure it out. That's why stylebot includes style guide citations! Go to http://www.webkit.org/coding/coding-style.html and look under "Comments" > I also looked for other examples in the changelog for API tests, but couldn't find any. Just say something like "Added API test 'mac/BackForwardList.mm'"
Comment on attachment 154171 [details] Patch Attachment 154171 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13345035 New failing tests: http/tests/navigation/success200-loadsame.html http/tests/navigation/success200-frames-loadsame.html
Created attachment 154190 [details] Archive of layout-test-results from gce-cr-linux-05 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment on attachment 154171 [details] Patch Removing r+ based on layout test failures.
Comment on attachment 154171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154171&action=review > Source/WebCore/loader/FrameLoader.cpp:-1239 > - // FIXME: is this the right place to reset loadType? Perhaps this should be done after loading is finished or aborted. > - m_loadType = FrameLoadTypeStandard; Is removing this FIXME correct? We're still reseting the loadtype up front, just inside loadWithDocumentLoader() instead? > Source/WebKit/mac/WebView/WebFrame.mm:1403 > + _private->coreFrame->loader()->load(request, substituteData, false, _private->coreFrame->loader()->loadType()); I'm also wondering if we really need to pass in the actual load type as an argument. Isn't it more of a boolean shouldReset yes/no, where in the latter case we re-use the previous load type? It just seems a bit weird to pass in "_private->coreFrame->loader()->loadType()", when calling load() on the same instance. If it is in fact a bool relationship, we might even be able to contain the magic of _when_ that is the case inside load(), though I'm not sure how. I assume that's what the original FIXME refers to.
Created attachment 154464 [details] Patch
Created attachment 154478 [details] Patch
Comment on attachment 154478 [details] Patch Attachment 154478 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13350429 New failing tests: http/tests/navigation/slowmetaredirect-basic.html
Created attachment 154505 [details] Archive of layout-test-results from gce-cr-linux-08 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
The failing test explicitly says in the comments about the test that the line that's missing in the test output after this patch shouldn't be in the output in the first place. It appears the test expectation might be wrong, and the output after this patch is correct.
Comment on attachment 154478 [details] Patch Attachment 154478 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13361252 New failing tests: http/tests/navigation/slowmetaredirect-basic.html
Created attachment 154519 [details] Archive of layout-test-results from gce-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 154754 [details] Patch Speculative fix for cr-linux EWS problems
Comment on attachment 154754 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154754&action=review > Tools/ChangeLog:13 > + * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: > + * TestWebKitAPI/Tests/mac/BackForwardList.mm: Added. > + (-[BackForwardListTest webView:didFinishLoadForFrame:]): > + (-[BackForwardListTest webView:didFailProvisionalLoadWithError:forFrame:]): > + (TestWebKitAPI): > + (TestWebKitAPI::TEST): BackForwardList.mm doesn't appear to be included in this diff...?
Created attachment 154756 [details] Patch Add missing file
Committed r123823: <http://trac.webkit.org/changeset/123823>