Issue 6883032: Print Preview: Do not show an invalid summary on load and remove the "no printers" UI code.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 6883032: Print Preview: Do not show an invalid summary on load and remove the "no printers" UI code. (Closed)

Created:
9 years, 8 months ago by Lei Zhang

Modified:
9 years, 7 months ago

Reviewers:
dpapad

CC:
chromium-reviews, arv (Not doing code reviews)

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Print Preview: Do not show an invalid summary on load and remove the "no printers" UI code. BUG=none TEST=No 'invalid page range message' on print preview startup; No more 'no printer' item in printer drop down. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82058

Patch Set 1 : '' #

Total comments: 2

Created: 9 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+13 lines, -25 lines)			Patch
M	chrome/app/generated_resources.grd	View	1 chunk	+0 lines, -3 lines	0 comments	Download
M	chrome/browser/resources/print_preview.js	View	6 chunks	+13 lines, -20 lines	2 comments	Download
M	chrome/browser/ui/webui/print_preview_ui_html_source.cc	View	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

Lei Zhang

http://codereview.chromium.org/6883032/diff/3001/chrome/browser/resources/print_preview.js File chrome/browser/resources/print_preview.js (right): http://codereview.chromium.org/6883032/diff/3001/chrome/browser/resources/print_preview.js#newcode123 chrome/browser/resources/print_preview.js:123: for (var i = 0; i < pageRanges.length; ++i) ...

9 years, 8 months ago (2011-04-19 01:29:17 UTC) #1

dpapad

9 years, 8 months ago (2011-04-19 01:50:36 UTC) #2

LGTM

http://codereview.chromium.org/6883032/diff/3001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview.js (left):

http://codereview.chromium.org/6883032/diff/3001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview.js:241: var option =
document.createElement('option');
Removing the "No printer found" is ok with me, since there is always going to be
at least one option. The idea behind that was to at least let the user know that
Chrome searched for any printers.

Expand Messages | Collapse Messages