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

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

Created:
11 years, 10 months ago by Evan Stade
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

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

Patch Set 1 #

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)
Evan Stade
I botched this up recently :(
11 years, 10 months ago (2009-02-18 22:37:27 UTC) #1
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...

Powered by Google App Engine
This is Rietveld 408576698