WebCore:
authortomernic <tomernic@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2006 22:30:39 +0000 (22:30 +0000)
committertomernic <tomernic@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2006 22:30:39 +0000 (22:30 +0000)
        Reviewed by Kevin Decker.

        Part of <rdar://problem/4351664> REGRESSION (420+): extra URL in b/f list - navigating back to previous page fails at apple.com/retail/)
        This also fixes <rdar://problem/4477821> REGRESSION (10.4.5-TOT): meta tag specifying refresh is being added to history.

        * bridge/mac/FrameMac.h:
        * bridge/mac/FrameMac.mm:
        Removed redirectionTimerFired().  This was added as attempt to fix <http://bugzilla.opendarwin.org/show_bug.cgi?id=7058>.  The
        aim was to cause Safari and WebKit to update their loading status after a redirect.  Unfortunately, the fix had a bad side
        effect.  Calling -reportClientRedirectCancelled: on a successful redirect causes WebKit to forget that the redirect was supposed
        to lock history (i.e. reuse the current back/forward entry for the new page).  The end result was that intermediate "quick" redirects
        were creating back/forward entries when they should not have been.  See 4351664.  That fix was almost correct, in that we do need to
        notify the frame load delegate when a redirect ends, either because it succeeded or because it was cancelled.  However, this is the
        wrong place to do it.  WebCore's redirect notification logic did not need to change to fix 7058.  The never-ending spinning indicators
        problem was actually caused by a bug at the WebKit level.

        * manual-tests/redirectHistory: Added.
        * manual-tests/redirectHistory/redir-1.html: Added.
        * manual-tests/redirectHistory/redir-2.html: Added.
        * manual-tests/redirectHistory/redir-3.html: Added.
        Manual test case.  I couldn't figure out how to create a layout test for this, because it involves navigation through history and
        it was unclear how/when to tell DumpRenderTree to dump its output.

WebKit:

        Reviewed by Kevin Decker.

        Better fix for <rdar://problem/4432562> REGRESSION (TOT): Safari's "stop loading" active, "view source" inactive after page load [7058]

        * WebView/WebFrame.m:
        (-[WebFrame _transitionToCommitted:]):
        Cancel the client redirect when we commit the provisional load, if we were waiting for a redirect.
        This is a better fix for 7058 (<rdar://problem/4432562>).  The original fix for 7058 changed the timing of the redirect cancel
        in such a way that WebKit was precluded from ever reusing back/forward list entries for redirects.  Clearing the redirect state
        here actually makes logical sense, as the redirect's target page is being committed at this point.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@13441 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bridge/mac/FrameMac.h
WebCore/bridge/mac/FrameMac.mm
WebCore/manual-tests/redirectHistory/redir-1.html [new file with mode: 0644]
WebCore/manual-tests/redirectHistory/redir-2.html [new file with mode: 0644]
WebCore/manual-tests/redirectHistory/redir-3.html [new file with mode: 0644]
WebKit/ChangeLog
WebKit/WebView/WebFrame.m

index aa452f1a917ada24239300bb7884d2d1fb52fc9a..676d528b816aa489c6c2b60bf54605216783a0af 100644 (file)
@@ -1,3 +1,28 @@
+2006-03-22  Tim Omernick  <timo@apple.com>
+
+        Reviewed by Kevin Decker.
+
+        Part of <rdar://problem/4351664> REGRESSION (420+): extra URL in b/f list - navigating back to previous page fails at apple.com/retail/)
+        This also fixes <rdar://problem/4477821> REGRESSION (10.4.5-TOT): meta tag specifying refresh is being added to history.
+
+        * bridge/mac/FrameMac.h:
+        * bridge/mac/FrameMac.mm:
+        Removed redirectionTimerFired().  This was added as attempt to fix <http://bugzilla.opendarwin.org/show_bug.cgi?id=7058>.  The
+        aim was to cause Safari and WebKit to update their loading status after a redirect.  Unfortunately, the fix had a bad side
+        effect.  Calling -reportClientRedirectCancelled: on a successful redirect causes WebKit to forget that the redirect was supposed
+        to lock history (i.e. reuse the current back/forward entry for the new page).  The end result was that intermediate "quick" redirects
+        were creating back/forward entries when they should not have been.  See 4351664.  That fix was almost correct, in that we do need to
+        notify the frame load delegate when a redirect ends, either because it succeeded or because it was cancelled.  However, this is the
+        wrong place to do it.  WebCore's redirect notification logic did not need to change to fix 7058.  The never-ending spinning indicators
+        problem was actually caused by a bug at the WebKit level.
+
+        * manual-tests/redirectHistory: Added.
+        * manual-tests/redirectHistory/redir-1.html: Added.
+        * manual-tests/redirectHistory/redir-2.html: Added.
+        * manual-tests/redirectHistory/redir-3.html: Added.
+        Manual test case.  I couldn't figure out how to create a layout test for this, because it involves navigation through history and
+        it was unclear how/when to tell DumpRenderTree to dump its output.
+
 2006-03-22  Eric Seidel  <eseidel@apple.com>
 
         Reviewed by darin.
index a5ef847db58a9353139530b6ec9454938d408583..b1c4691df765c35e81b4d6504a32cf06884c37bd 100644 (file)
@@ -309,7 +309,6 @@ public:
 protected:
     virtual void startRedirectionTimer();
     virtual void stopRedirectionTimer();
-    virtual void redirectionTimerFired(Timer<Frame>*);
 
 private:
     virtual void handleMousePressEvent(const MouseEventWithHitTestResults&);
index 5a27a2e3e3ee17260a579f445fed12607ad55004..2fbda60b20b7920bbf296302c99fe76b1914d23b 100644 (file)
@@ -837,20 +837,6 @@ void FrameMac::stopRedirectionTimer()
         [_bridge reportClientRedirectCancelled:d->m_cancelWithLoadInProgress];
 }
 
-void FrameMac::redirectionTimerFired(Timer<Frame>* timer)
-{
-    // Note, despite its name, we must call "redirect cancelled" even when the
-    // redirect has completed successfully. Although that may not have been our
-    // original intent, Safari depends on this behavior at the time of this writing.
-    // See Radar 4432562 and Bugzilla 7058.
-
-    // Don't report history navigations, just actual redirection.
-    if (d->m_scheduledRedirection != historyNavigationScheduled)
-        [_bridge reportClientRedirectCancelled:d->m_cancelWithLoadInProgress];
-
-    Frame::redirectionTimerFired(timer);
-}
-
 String FrameMac::userAgent() const
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
diff --git a/WebCore/manual-tests/redirectHistory/redir-1.html b/WebCore/manual-tests/redirectHistory/redir-1.html
new file mode 100644 (file)
index 0000000..1cdab45
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<head>
+<meta http-equiv="refresh" content="3;url=redir-2.html"/>
+<title>Redirect 1</title>
+</head>
+<body>
+<p>This test checks that a &quot;quick&quot; redirect does not create an extra entry in the back/forward list.  See Radar 4351664.</p>
+<p>Wait a few seconds for the redirect to occur.  If you return to this page after the redirect, then the test passed.  If you end up at redir-2.html, then the test failed.</p>
+</body>
+</html>
diff --git a/WebCore/manual-tests/redirectHistory/redir-2.html b/WebCore/manual-tests/redirectHistory/redir-2.html
new file mode 100644 (file)
index 0000000..fe9e6ba
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<head>
+<meta http-equiv="refresh" content="0;url=redir-3.html"/>
+<title>Redirect 2</title>
+</head>
+<body>
+<p>Test failed</p>
+</body>
+</html>
diff --git a/WebCore/manual-tests/redirectHistory/redir-3.html b/WebCore/manual-tests/redirectHistory/redir-3.html
new file mode 100644 (file)
index 0000000..a525851
--- /dev/null
@@ -0,0 +1,11 @@
+<html>
+<head>
+<title>Redirect 3</title>
+<script>
+window.setTimeout("window.history.back()", 1000);
+</script>
+</head>
+<body>
+<p>Going back...</p>
+</body>
+</html>
index 419de843a9d5eee56d3c40be502ac61a35d85646..e518c9e412e3bf7122572905403a4d13840ae696 100644 (file)
@@ -1,3 +1,16 @@
+2006-03-22  Tim Omernick  <timo@apple.com>
+
+        Reviewed by Kevin Decker.
+
+        Better fix for <rdar://problem/4432562> REGRESSION (TOT): Safari's "stop loading" active, "view source" inactive after page load [7058]
+
+        * WebView/WebFrame.m:
+        (-[WebFrame _transitionToCommitted:]):
+        Cancel the client redirect when we commit the provisional load, if we were waiting for a redirect.
+        This is a better fix for 7058 (<rdar://problem/4432562>).  The original fix for 7058 changed the timing of the redirect cancel
+        in such a way that WebKit was precluded from ever reusing back/forward list entries for redirects.  Clearing the redirect state
+        here actually makes logical sense, as the redirect's target page is being committed at this point.
+
 2006-03-21  Darin Adler  <darin@apple.com>
 
         - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=3784
index 5bd499af73106735f395c639e6093eaf33caeac1..6d47b42c8d2c33e0acac9638f8b1b9103c4aad61 100644 (file)
@@ -778,6 +778,13 @@ static inline WebFrame *Frame(WebCoreFrameBridge *bridge)
                         // We must also clear out form data so we don't try to restore it into the incoming page,
                         // see -_opened
                     }
+
+                    // This dataSource was a client redirect (we checked above, remember?).  Now that it is being
+                    // committed, there can no longer be a pending redirect.  Call -_clientRedirectCancelled: here
+                    // so that the frame load delegate is notified that the redirect's status has changed.  The
+                    // frame load delegate may have saved some state about the redirect in its
+                    // -webView:willPerformClientRedirectToURL:delay:fireDate:forFrame:.
+                    [self _clientRedirectCancelled:NO];
                 }
                 [self _makeDocumentView];
                 break;