Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(47)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 7 months ago by Evan Stade
Modified:
6 years, 3 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
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 5 (0 generated)
Evan Stade
I botched this up recently :(
8 years, 7 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. ...
8 years, 7 months ago (2009-02-18 22:45:27 UTC) #2
darin (slow to review)
Feel free to cut the test as a separate CL.
8 years, 7 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 ...
8 years, 7 months ago (2009-02-18 22:46:23 UTC) #4
tony
8 years, 7 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b