Summary: | Reloading substitute-data/alternate html string for unreachableURL will add an item to the back-forward-history for each reload | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tor Arne Vestbø <vestbo> | ||||||||||||||||||||
Component: | History | Assignee: | Vicki Pfau <jeffrey+webkit> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | abarth, ap, beidson, ddkilzer, dglazkov, japhet, jeffrey+webkit, jochen, sam, webkit.review.bot | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Attachments: |
|
Description
Tor Arne Vestbø
2012-04-16 09:31:29 PDT
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> |