Issue 13689002: LabelButton: SetAccessibleName on SetText; update test.

Issue 13689002: LabelButton: SetAccessibleName on SetText; update test. (Closed)

Created:
7 years, 8 months ago by msw

Modified:
7 years, 8 months ago

Reviewers:
tfarina, sky

CC:
chromium-reviews, hashimoto+watch_chromium.org, aboxhall+watch_chromium.org, tfarina, yoshiki+watch_chromium.org, yuzo+watch_chromium.org, davidbarr+watch_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, ctguil+watch_chromium.org, zork+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

LabelButton: SetAccessibleName on SetText; update test. SetAccessibleName to label text on SetText. (this matches TextButtonBase::SetText behavior) Call SetText from LabelButton's constructor. Use LabelButton in AccessibilityEventRouterViewsTest like: https://codereview.chromium.org/13584010 Patch Set 4. Note: This CL depends on https://codereview.chromium.org/13639003/ BUG=155363, 226814 TEST=LabelButtons have accessibility names. Tests pass. R=tfarina@chromium.org,sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192557

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebase on focusable change from crrev.com/192519 #

Total comments: 2

Created: 7 years, 8 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+14 lines, -7 lines)			Patch
	M	chrome/browser/ui/views/accessibility/accessibility_event_router_views_unittest.cc	View		4 chunks	+11 lines, -6 lines	0 comments	Download
	M	ui/views/controls/button/label_button.cc	View	1	2 chunks	+3 lines, -1 line	2 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

msw

Hey Thiago and Scott, please take a look; thanks! https://codereview.chromium.org/13689002/diff/1/ui/views/controls/button/label_button.cc File ui/views/controls/button/label_button.cc (right): https://codereview.chromium.org/13689002/diff/1/ui/views/controls/button/label_button.cc#newcode130 ui/views/controls/button/label_button.cc:130: ...

7 years, 8 months ago (2013-04-05 01:06:18 UTC) #1

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/13689002/7001

7 years, 8 months ago (2013-04-05 09:09:35 UTC) #3

tfarina

lgtm https://chromiumcodereview.appspot.com/13689002/diff/7001/ui/views/controls/button/label_button.cc File ui/views/controls/button/label_button.cc (right): https://chromiumcodereview.appspot.com/13689002/diff/7001/ui/views/controls/button/label_button.cc#newcode38 ui/views/controls/button/label_button.cc:38: label_(new Label()), you don't pass it through constructor ...

7 years, 8 months ago (2013-04-05 12:44:27 UTC) #4

msw

7 years, 8 months ago (2013-04-05 15:04:17 UTC) #6

Message was sent while issue was closed.

https://chromiumcodereview.appspot.com/13689002/diff/7001/ui/views/controls/b...
File ui/views/controls/button/label_button.cc (right):

https://chromiumcodereview.appspot.com/13689002/diff/7001/ui/views/controls/b...
ui/views/controls/button/label_button.cc:38: label_(new Label()),
On 2013/04/05 12:44:27, tfarina wrote:
> you don't pass it through constructor anymore, because you call SetText()
below,
> right?

yes

Expand Messages | Collapse Messages