Issue 249523002: Add extra column to details of signin-internals.

Issue 249523002: Add extra column to details of signin-internals. (Closed)

Created:
6 years, 8 months ago by Mike Lerman

Modified:
6 years, 7 months ago

Reviewers:
Roger Tawa OOO till Jul 10th, Evan Stade

CC:
chromium-reviews, arv+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Add extra column to details of signin-internals. BUG=326577 TEST=Each detail how has both the time and status on one row rather than split over two rows. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266502

Patch Set 1 #

Total comments: 2

Patch Set 2 : Change variable names for the better #

Patch Set 3 : Pretty up the HTML table and the variable names. #

Created: 6 years, 8 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+20 lines, -26 lines)			Patch
	M	chrome/browser/resources/signin_internals/signin_index.html	View	1 2	1 chunk	+3 lines, -1 line	0 comments	Download
	M	components/signin/core/browser/about_signin_internals.cc	View	1 2	3 chunks	+17 lines, -25 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Mike Lerman

Could I get a review on this change? The upper-right hand pane of the chrome://signin-internals ...

6 years, 8 months ago (2014-04-23 18:01:16 UTC) #1

Evan Stade

lgtm with complaint https://codereview.chromium.org/249523002/diff/1/components/signin/core/browser/about_signin_internals.cc File components/signin/core/browser/about_signin_internals.cc (right): https://codereview.chromium.org/249523002/diff/1/components/signin/core/browser/about_signin_internals.cc#newcode50 components/signin/core/browser/about_signin_internals.cc:50: entry->SetString("value2", field_val2); "value"/"value2" is perhaps the ...

6 years, 8 months ago (2014-04-24 00:36:46 UTC) #2

Mike Lerman

I also made the HTML table a bit prettier (so content-less cells exist). https://codereview.chromium.org/249523002/diff/1/components/signin/core/browser/about_signin_internals.cc File ...

6 years, 8 months ago (2014-04-24 15:53:58 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlerman@chromium.org/249523002/40001

6 years, 8 months ago (2014-04-25 22:18:45 UTC) #6

Message was sent while issue was closed.

Change committed as 266502

Expand Messages | Collapse Messages