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

Issue 7108054: Revert 88611 - Print Preview: Changing displayed error message when PDF Viewer is missing. (Closed)

Created:
9 years, 6 months ago by ericu
Modified:
9 years, 6 months ago
Reviewers:
dpapad
CC:
chromium-reviews, arv (Not doing code reviews)
Visibility:
Public.

Description

Revert 88611 - Print Preview: Changing displayed error message when PDF Viewer is missing. BUG=85383 TEST=Launch print preview without having the plugin. A new error message should be displayed, including a button offering to launch the native print dialog. Review URL: http://codereview.chromium.org/7003089 TBR=dpapad@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88613

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -33 lines) Patch
M chrome/app/chromium_strings.grd View 1 chunk +1 line, -4 lines 0 comments Download
M chrome/browser/resources/print_preview.html View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/print_preview.js View 4 chunks +14 lines, -26 lines 0 comments Download
M chrome/browser/ui/webui/print_preview_data_source.cc View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
ericu
9 years, 6 months ago (2011-06-10 00:17:30 UTC) #1
ericu
9 years, 6 months ago (2011-06-10 00:21:09 UTC) #2
See
http://chromegw.corp.google.com/i/chromium/builders/Mac/builds/4766/steps/com...;

In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:107:1:error:
"IDS_APP_MENU_PRODUCT_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:108:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:108:1:error:
"IDS_HELPER_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:109:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:109:1:error:
"IDS_SHORT_HELPER_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:110:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:110:1:error:
"IDS_RENDERER_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:111:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:111:1:error:
"IDS_SHORT_RENDERER_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:112:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:112:1:error:
"IDS_PLUGIN_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:113:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:113:1:error:
"IDS_SHORT_PLUGIN_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:114:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:114:1:error:
"IDS_WORKER_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:115:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:115:1:error:
"IDS_SHORT_WORKER_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:116:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:116:1:error:
"IDS_UTILITY_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:117:1:error:
this is the location of the previous definition
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:20:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/google_chrome_strings.h:117:1:error:
"IDS_SHORT_UTILITY_APP_NAME" redefined
In file included from
/b/build/slave/cr-mac-rel/build/src/chrome/browser/ui/webui/print_preview_data_source.cc:18:
/b/build/slave/cr-mac-rel/build/src/chrome/../xcodebuild/DerivedSources/Release/chrome/grit/chromium_strings.h:118:1:error:
this is the location of the previous definition


On Thu, Jun 9, 2011 at 5:17 PM,  <ericu@chromium.org> wrote:
> Reviewers: dpapad,
>
> Description:
> Revert 88611 - Print Preview: Changing displayed error message when PDF
> Viewer
> is missing.
>
> BUG=85383
> TEST=Launch print preview without having the plugin. A new error message
> should
> be displayed,
> including a button offering to launch the native print dialog.
>
>
> Review URL: http://codereview.chromium.org/7003089
>
> TBR=dpapad@chromium.org
>
> Please review this at http://codereview.chromium.org/7108054/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome/app/chromium_strings.grd
>  M     chrome/browser/resources/print_preview.html
>  M     chrome/browser/resources/print_preview.js
>  M     chrome/browser/ui/webui/print_preview_data_source.cc
>
>
> Index: chrome/app/chromium_strings.grd
> ===================================================================
> --- chrome/app/chromium_strings.grd     (revision 88612)
> +++ chrome/app/chromium_strings.grd     (working copy)
> @@ -530,11 +530,8 @@
>       </message>
>       <!-- Print Preview -->
>       <message name="IDS_PRINT_PREVIEW_NO_PLUGIN" desc="Message to display
> when the PDF viewer is missing.">
> -        Chromium does not include the PDF viewer which is required for
> Print Preview to function.
> +      Sorry! Chromium does not include the PDF viewer built into Google
> Chrome, which is required to show the print preview.
>       </message>
> -      <message name="IDS_PRINT_PREVIEW_NATIVE_DIALOG" desc="Option offering
> the user to open the native print dialog, displayed when the PDF viewer is
> missing.">
> -        Launch native print dialog
> -      </message>
>
>       <if expr="os == 'darwin'">
>         <message name="IDS_APP_MENU_PRODUCT_NAME" desc="The application's
> short name, used for the Mac's application menu, activity monitor, etc. This
> should be less than 16 characters. Example: Chrome, not Google Chrome.">
> Index: chrome/browser/resources/print_preview.html
> ===================================================================
> --- chrome/browser/resources/print_preview.html (revision 88612)
> +++ chrome/browser/resources/print_preview.html (working copy)
> @@ -119,7 +119,8 @@
>         </div>
>         <div id="error-text" class="hidden"></div>
>         <br>
> -        <button id="error-button" class="hidden"></button>
> +        <button id="reopen-page" class="hidden"
> +            i18n-content="reopenPage"></button>
>       </div>
>     </div>
>   </div>
> Index: chrome/browser/resources/print_preview.js
> ===================================================================
> --- chrome/browser/resources/print_preview.js   (revision 88612)
> +++ chrome/browser/resources/print_preview.js   (working copy)
> @@ -42,9 +42,7 @@
>   $('cancel-button').addEventListener('click', handleCancelButtonClick);
>
>   if (!checkCompatiblePluginExists()) {
> -    displayErrorMessageWithButton(localStrings.getString('noPlugin'),
> -
>  localStrings.getString('launchNativeDialog'),
> -                                  showSystemDialog);
> +    displayErrorMessage(localStrings.getString('noPlugin'), false);
>     $('mainview').parentElement.removeChild($('dummy-viewer'));
>     return;
>   }
> @@ -143,10 +141,10 @@
>  * @param {string} initiatorTabURL The URL of the initiator tab.
>  */
>  function onInitiatorTabClosed(initiatorTabURL) {
> -  displayErrorMessageWithButton(
> -      localStrings.getString('initiatorTabClosed'),
> -      localStrings.getString('reopenPage'),
> -      function() { window.location = initiatorTabURL; });
> +  $('reopen-page').addEventListener('click', function() {
> +      window.location = initiatorTabURL;
> +  });
> +  displayErrorMessage(localStrings.getString('initiatorTabClosed'), true);
>  }
>
>  /**
> @@ -439,12 +437,19 @@
>  /**
>  * Display an error message in the center of the preview area.
>  * @param {string} errorMessage The error message to be displayed.
> + * @param {boolean} showButton Indivates whether the "Reopen the page"
> button
> + * should be displayed.
>  */
> -function displayErrorMessage(errorMessage) {
> +function displayErrorMessage(errorMessage, showButton) {
>   $('overlay-layer').classList.remove('invisible');
>   $('dancing-dots-text').classList.add('hidden');
>   $('error-text').innerHTML = errorMessage;
>   $('error-text').classList.remove('hidden');
> +  if (showButton)
> +    $('reopen-page').classList.remove('hidden');
> +  else
> +    $('reopen-page').classList.add('hidden');
> +
>   removeEventListeners();
>   var pdfViewer = $('pdf-viewer');
>   if (pdfViewer)
> @@ -452,28 +457,11 @@
>  }
>
>  /**
> - * Display an error message in the center of the preview area followed by a
> - * button.
> - * @param {string} errorMessage The error message to be displayed.
> - * @param {string} buttonText The text to be displayed within the button.
> - * @param {string} buttonListener The listener to be executed when the
> button is
> - * clicked.
> - */
> -function displayErrorMessageWithButton(
> -    errorMessage, buttonText, buttonListener) {
> -  var errorButton = $('error-button');
> -  errorButton.innerHTML = buttonText;
> -  errorButton.onclick = buttonListener;
> -  errorButton.classList.remove('hidden');
> -  displayErrorMessage(errorMessage);
> -}
> -
> -/**
>  * Display an error message when print preview fails.
>  * Called from PrintPreviewMessageHandler::OnPrintPreviewFailed().
>  */
>  function printPreviewFailed() {
> -  displayErrorMessage(localStrings.getString('previewFailed'));
> +  displayErrorMessage(localStrings.getString('previewFailed'), false);
>  }
>
>  /**
> Index: chrome/browser/ui/webui/print_preview_data_source.cc
> ===================================================================
> --- chrome/browser/ui/webui/print_preview_data_source.cc        (revision
> 88612)
> +++ chrome/browser/ui/webui/print_preview_data_source.cc        (working
> copy)
> @@ -36,8 +36,6 @@
>   localized_strings->SetString(std::string("noPlugin"),
>       l10n_util::GetStringUTF8(IDS_PRINT_PREVIEW_NO_PLUGIN));
>  #endif
> -  localized_strings->SetString(std::string("launchNativeDialog"),
> -      l10n_util::GetStringUTF8(IDS_PRINT_PREVIEW_NATIVE_DIALOG));
>   localized_strings->SetString(std::string("previewFailed"),
>       l10n_util::GetStringUTF8(IDS_PRINT_PREVIEW_FAILED));
>   localized_strings->SetString(std::string("initiatorTabClosed"),
>
>
>

Powered by Google App Engine
This is Rietveld 408576698