Print Preview: Changing error message when initiator tab closes.

Print Preview: Changing error message when initiator tab closes.

Adding a button instead of a link to allow the user to reopen the initiator page.

BUG=83244
TEST=See bug description


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86321

[Lei Zhang, 2011-05-23 07:07:04]

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources....
chrome/app/generated_resources.grd:5568: <message
name="IDS_PRINT_PREVIEW_INITIATOR_TAB_CLOSED" desc="Message to display when
print preview fails, because the user closed the tab that initiated the print
preview before the preview was generated.">
nit: update this description too.

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources....
chrome/app/generated_resources.grd:5571: <message
name="IDS_PRINT_PREVIEW_REOPEN_PAGE" desc="Option offering the user to reopen
the page that was being previewed initially.">
how about better description? Something like:

Text for button displayed when print preview fails because the user closed the
tab that initiated the print preview. Clicking the button reopens the page on
the tab that was close.

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
File chrome/browser/resources/print_preview.js (right):

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:104:
$('reopen-page').addEventListener('click', function() {
Show this go in onload()?

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:388: * @param (string) errorMessage
The error message to be displayed.
update comment.

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:397:
$('reopen-page').classList.remove('hidden');
can we ever get into a situation where displayErrorMessage() gets called once
with |showButton| = true and then again with |showButton| = false? If yes, then
add a else case to hide the button.

[dpapad, 2011-05-23 16:05:31]

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources....
chrome/app/generated_resources.grd:5568: <message
name="IDS_PRINT_PREVIEW_INITIATOR_TAB_CLOSED" desc="Message to display when
print preview fails, because the user closed the tab that initiated the print
preview before the preview was generated.">
On 2011/05/23 07:07:04, Lei Zhang wrote:
> nit: update this description too.

Done.

http://codereview.chromium.org/7051026/diff/7/chrome/app/generated_resources....
chrome/app/generated_resources.grd:5571: <message
name="IDS_PRINT_PREVIEW_REOPEN_PAGE" desc="Option offering the user to reopen
the page that was being previewed initially.">
On 2011/05/23 07:07:04, Lei Zhang wrote:
> how about better description? Something like:
> 
> Text for button displayed when print preview fails because the user closed the
> tab that initiated the print preview. Clicking the button reopens the page on
> the tab that was close.

Done.

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
File chrome/browser/resources/print_preview.js (right):

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:104:
$('reopen-page').addEventListener('click', function() {
On 2011/05/23 07:07:04, Lei Zhang wrote:
> Show this go in onload()?

It cant go in onload() because |initiatorTabURL| is not known at that time. This
function is directly called from c++ code. Also I prefer not introducing another
global variable. Adding the listener right before showing the button seems ok to
me.

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:388: * @param (string) errorMessage
The error message to be displayed.
On 2011/05/23 07:07:04, Lei Zhang wrote:
> update comment.

Done.

http://codereview.chromium.org/7051026/diff/7/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:397:
$('reopen-page').classList.remove('hidden');
On 2011/05/23 07:07:04, Lei Zhang wrote:
> can we ever get into a situation where displayErrorMessage() gets called once
> with |showButton| = true and then again with |showButton| = false? If yes,
then
> add a else case to hide the button.

I cant think of such a scenario, but I will add the else statement just to be
safe.

[Lei Zhang, 2011-05-23 18:16:21]

LGTM

[commit-bot: I haz the power, 2011-05-23 19:25:15]

Change committed as 86321