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

Issue 48138: Bug 4688: First run bubble ignores user's keyboard attempt... (Closed)

Created:
11 years, 9 months ago by Finnur
Modified:
9 years, 6 months ago
Reviewers:
jcampan
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Bug 4688: First run bubble ignores user's keyboard attempt (tab) to change Search Engine. Not a good first impression. BUG=4688 TEST=Open Chrome using --first-run, press Cancel on the import dialog and watch the First Run bubble come up. Use Tab and then Enter and the Change Search Engine should appear. Then try it again, but this time click outside the bubble to dismiss it (corner case that could lead to a crash). Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12014

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 1

Patch Set 4 : '' #

Total comments: 2

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -10 lines) Patch
M chrome/browser/views/first_run_bubble.h View 1 2 3 4 chunks +12 lines, -5 lines 0 comments Download
M chrome/browser/views/first_run_bubble.cc View 1 2 3 4 6 chunks +31 lines, -5 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Finnur
As a break from layout tests I decided to tackle an easy 2.0 problem (which ...
11 years, 9 months ago (2009-03-18 17:57:21 UTC) #1
jcampan
http://codereview.chromium.org/48138/diff/1004/6 File chrome/browser/views/first_run_bubble.cc (right): http://codereview.chromium.org/48138/diff/1004/6#newcode243 Line 243: view_ = new FirstRunBubbleView(window, profile); Instead of using ...
11 years, 9 months ago (2009-03-18 19:17:22 UTC) #2
Finnur
That's not a bad idea. Updated. On 2009/03/18 19:17:22, jcampan wrote: > http://codereview.chromium.org/48138/diff/1004/6 > File ...
11 years, 9 months ago (2009-03-18 19:50:30 UTC) #3
jcampan
http://codereview.chromium.org/48138/diff/1008/1009 File chrome/browser/views/first_run_bubble.cc (right): http://codereview.chromium.org/48138/diff/1008/1009#newcode184 Line 184: static_cast<views::NativeButton*>(focused_now); May be you could check the class ...
11 years, 9 months ago (2009-03-18 20:03:10 UTC) #4
Finnur
> I am just afraid that if we later on add a > non nativebutton ...
11 years, 9 months ago (2009-03-18 20:22:51 UTC) #5
jcampan
11 years, 9 months ago (2009-03-18 21:11:19 UTC) #6
LGTM

Powered by Google App Engine
This is Rietveld 408576698