Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(248)

Issue 67150: Make the Go Back navigation work even when redirection is involved.... (Closed)

Created:
11 years, 8 months ago by yuzo
Modified:
9 years, 7 months ago
Reviewers:
brettw
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Make the Go Back navigation work even when redirection is involved. Currently, Chrome tries to go back to the page immediately before the current one. This doesn't work if the current page was visited by redirection; redirection just occurs again. With this change, Chrome first tries to find the redirection source of the current page and then to go back to the page before the source. BUG=9663, 10531 Tested: unit_tests, ui_tests, manually. (unit_tests fails for URLFetcherBadHTTPSTest.BadHTTPSTest, but I believe it is unrelated to this change.)

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -3 lines) Patch
M chrome/browser/tab_contents/navigation_controller.h View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/navigation_controller.cc View 1 2 3 3 chunks +22 lines, -3 lines 1 comment Download

Messages

Total messages: 8 (0 generated)
yuzo
Hi, Brett, Can you review this? Yuzo
11 years, 8 months ago (2009-04-15 00:10:26 UTC) #1
brettw
This seems wrong to me. It will only work when you press back, the back/forward ...
11 years, 8 months ago (2009-04-15 17:19:59 UTC) #2
yuzo
Hi, Brett, Thank you for the review. The back item in the popup menu works ...
11 years, 8 months ago (2009-04-15 18:19:39 UTC) #3
yuzo
OK, I'll try your approach and see how it goes. Yuzo On 2009/04/15 18:19:39, yuzo ...
11 years, 8 months ago (2009-04-15 19:10:24 UTC) #4
yuzo
Brett, You were right. Your approach is actually simpler and also fixes Bug 10531. Much ...
11 years, 8 months ago (2009-04-15 21:18:07 UTC) #5
yuzo
Brett, I read the code and it seems we should remove the current entry only ...
11 years, 8 months ago (2009-04-21 02:44:10 UTC) #6
brettw
LGTM with this one style nit. Thanks for working on this! http://codereview.chromium.org/67150/diff/3001/4001 File chrome/browser/tab_contents/navigation_controller.cc (right): ...
11 years, 8 months ago (2009-04-24 20:57:59 UTC) #7
yuzo
11 years, 7 months ago (2009-05-01 05:05:52 UTC) #8
This has been superseded with http://codereview.chromium.org/100245

Please review it instead.

On 2009/04/24 20:57:59, brettw wrote:
> LGTM with this one style nit. Thanks for working on this!
> 
> http://codereview.chromium.org/67150/diff/3001/4001
> File chrome/browser/tab_contents/navigation_controller.cc (right):
> 
> http://codereview.chromium.org/67150/diff/3001/4001#newcode825
> Line 825: void NavigationController::InsertOrReplaceEntry(
> When all the args fit, we usually like to wrap the args like this:
> 
> ...laceEntry(NavigationEntry* entry,
>              bool replace) {

Powered by Google App Engine
This is Rietveld 408576698