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

Issue 3080039: Autofill: Two small fixes. (Closed)

Created:
10 years, 4 months ago by tfarina
Modified:
9 years, 7 months ago
Reviewers:
James Hawkins, sky
CC:
chromium-reviews, ben+cc_chromium.org
Visibility:
Public.

Description

Autofill: Two small fixes. - Remove unnecessary "Summary" line from Autofill options dialog box. - Make the sub-dialogs ('Add an address' and 'Add a credit card') modal. BUG=49881, 49787 TEST=Go to Options > Personal Stuff > AutoFill Options. The listview shouldn't have any header. Click on 'Add address...' or in 'Add credit card...', try to come back to the AutoFill options dialog again, it shouldn't be possible. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55527

Patch Set 1 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -19 lines) Patch
M chrome/browser/views/autofill_profiles_view_win.h View 11 chunks +11 lines, -14 lines 1 comment Download
M chrome/browser/views/autofill_profiles_view_win.cc View 1 chunk +1 line, -5 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
tfarina
10 years, 4 months ago (2010-08-07 17:11:06 UTC) #1
James Hawkins
http://codereview.chromium.org/3080039/diff/9002/10002 File chrome/browser/views/autofill_profiles_view_win.h (right): http://codereview.chromium.org/3080039/diff/9002/10002#newcode261 chrome/browser/views/autofill_profiles_view_win.h:261: virtual bool IsModal() const { return true; } What ...
10 years, 4 months ago (2010-08-09 16:45:49 UTC) #2
tfarina
On Mon, Aug 9, 2010 at 1:45 PM, <jhawkins@chromium.org> wrote: > > http://codereview.chromium.org/3080039/diff/9002/10002 > File ...
10 years, 4 months ago (2010-08-09 16:51:57 UTC) #3
James Hawkins
10 years, 4 months ago (2010-08-09 17:18:27 UTC) #4
On 2010/08/09 16:51:57, tfarina wrote:
> On Mon, Aug 9, 2010 at 1:45 PM,  <mailto:jhawkins@chromium.org> wrote:
> >
> > http://codereview.chromium.org/3080039/diff/9002/10002
> > File chrome/browser/views/autofill_profiles_view_win.h (right):
> >
> > http://codereview.chromium.org/3080039/diff/9002/10002#newcode261
> > chrome/browser/views/autofill_profiles_view_win.h:261: virtual bool
> > IsModal() const { return true; }
> > What is the impetus for this change?
> >
> 
> You said in the bug:
> "It sounds like the problem is that the sub-dialog is non-modal when
> it should be modal."
> 
> Quoting the principal: "it should be modal".
> 
> So that was the motivation.
> 
> Why it sounds wrong?
> 

It doesn't, just needed a refresher :-)

LGTM

Powered by Google App Engine
This is Rietveld 408576698