Issue 21482: Fix save complete .html page.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 21482: Fix save complete .html page. (Closed)

Created:
11 years, 10 months ago by Evan Stade

Modified:
9 years, 6 months ago

Reviewers:
tony, darin (slow to review), Venkat Yellapu

CC:
chromium-reviews_googlegroups.com

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Fix save complete .html page. BUG=7616 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9988

Patch Set 1 #

Created: 11 years, 10 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+4 lines, -11 lines)			Patch
	M	chrome/browser/download/save_package.cc	View		1 chunk	+4 lines, -11 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

darin (slow to review)

LGTM However, I think you have identified a case where we are missing a test. ...

11 years, 10 months ago (2009-02-18 22:45:27 UTC) #2

darin (slow to review)

Feel free to cut the test as a separate CL.

11 years, 10 months ago (2009-02-18 22:45:46 UTC) #3

Evan Stade

On 2009/02/18 22:45:27, darin wrote: > LGTM > > However, I think you have identified ...

11 years, 10 months ago (2009-02-18 22:46:23 UTC) #4

tony

11 years, 10 months ago (2009-02-18 22:54:24 UTC) #5

There actually is a UI test for this.  However, the if block right above this
code is where the test skips.  The call to win_util::SaveFileAsWithFilter is
what brings up the save prompt, so this call was skipped.  The bug was in this
method call :(

I guess we could test it by providing a mock win_util::SaveFileAsWithFilter?

On 2009/02/18 22:45:27, darin wrote:
> LGTM
> 
> However, I think you have identified a case where we are missing a test.  It
is
> bad that no tests caught this mistake.  Can you create a test?  Maybe by
> extending save_page_uitest.cc...

Expand Messages | Collapse Messages