Commit 255e45ad authored by Daniel Hosseinian's avatar Daniel Hosseinian Committed by Chromium LUCI CQ

PDF Viewer: Optimize and cleanup password dialog

Wrap around a dom-if template, which is a thriftier use of resources
since the password dialog should only be shown at the beginning of a
PDF viewing session. More importantly, only a small portion of PDFs
require the dialog.

Rename the element from viewer-password-screen to
viewer-password-dialog.

Fetch password dialog screens before the page is served instead of
using load time data.

Change-Id: I240b8c416d51e423e177548d773eab98e4ca360a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632265Reviewed-by: default avatardpapad <dpapad@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845210}
parent 030dfd2d
...@@ -60,7 +60,7 @@ preprocess_if_expr("preprocess_generated") { ...@@ -60,7 +60,7 @@ preprocess_if_expr("preprocess_generated") {
"elements/viewer-download-controls.js", "elements/viewer-download-controls.js",
"elements/viewer-error-screen.js", "elements/viewer-error-screen.js",
"elements/viewer-page-selector.js", "elements/viewer-page-selector.js",
"elements/viewer-password-screen.js", "elements/viewer-password-dialog.js",
"elements/viewer-pdf-sidenav.js", "elements/viewer-pdf-sidenav.js",
"elements/viewer-pdf-toolbar.js", "elements/viewer-pdf-toolbar.js",
"elements/viewer-pdf-toolbar-new.js", "elements/viewer-pdf-toolbar-new.js",
...@@ -319,7 +319,7 @@ js_library("pdf_viewer") { ...@@ -319,7 +319,7 @@ js_library("pdf_viewer") {
":pdf_viewer_utils", ":pdf_viewer_utils",
":toolbar_manager", ":toolbar_manager",
"elements:viewer-error-screen", "elements:viewer-error-screen",
"elements:viewer-password-screen", "elements:viewer-password-dialog",
"elements:viewer-pdf-sidenav", "elements:viewer-pdf-sidenav",
"elements:viewer-pdf-toolbar", "elements:viewer-pdf-toolbar",
"elements:viewer-pdf-toolbar-new", "elements:viewer-pdf-toolbar-new",
......
...@@ -15,7 +15,7 @@ js_type_check("closure_compile") { ...@@ -15,7 +15,7 @@ js_type_check("closure_compile") {
":viewer-error-screen", ":viewer-error-screen",
":viewer-page-indicator", ":viewer-page-indicator",
":viewer-page-selector", ":viewer-page-selector",
":viewer-password-screen", ":viewer-password-dialog",
":viewer-pdf-sidenav", ":viewer-pdf-sidenav",
":viewer-pdf-toolbar", ":viewer-pdf-toolbar",
":viewer-pdf-toolbar-new", ":viewer-pdf-toolbar-new",
...@@ -110,10 +110,11 @@ js_library("viewer-page-indicator") { ...@@ -110,10 +110,11 @@ js_library("viewer-page-indicator") {
js_library("viewer-page-selector") { js_library("viewer-page-selector") {
} }
js_library("viewer-password-screen") { js_library("viewer-password-dialog") {
deps = [ deps = [
"//ui/webui/resources/cr_elements/cr_button:cr_button.m",
"//ui/webui/resources/cr_elements/cr_dialog:cr_dialog.m",
"//ui/webui/resources/cr_elements/cr_input:cr_input.m", "//ui/webui/resources/cr_elements/cr_input:cr_input.m",
"//ui/webui/resources/js:load_time_data.m",
] ]
} }
...@@ -211,7 +212,7 @@ html_to_js("web_components") { ...@@ -211,7 +212,7 @@ html_to_js("web_components") {
"viewer-error-screen.js", "viewer-error-screen.js",
"viewer-page-indicator.js", "viewer-page-indicator.js",
"viewer-page-selector.js", "viewer-page-selector.js",
"viewer-password-screen.js", "viewer-password-dialog.js",
"viewer-pdf-sidenav.js", "viewer-pdf-sidenav.js",
"viewer-pdf-toolbar.js", "viewer-pdf-toolbar.js",
"viewer-pdf-toolbar-new.js", "viewer-pdf-toolbar-new.js",
......
...@@ -3,14 +3,12 @@ ...@@ -3,14 +3,12 @@
margin-top: var(--cr-form-field-bottom-spacing); margin-top: var(--cr-form-field-bottom-spacing);
} }
</style> </style>
<cr-dialog id="dialog" no-cancel> <cr-dialog id="dialog" no-cancel show-on-attach>
<div slot="title">$i18n{passwordDialogTitle}</div> <div slot="title">$i18n{passwordDialogTitle}</div>
<div slot="body"> <div slot="body">
<div id="message">$i18n{passwordPrompt}</div> <div id="message">$i18n{passwordPrompt}</div>
<cr-input id="password" <cr-input id="password" type="password"
type="password" error-message="$i18n{passwordInvalid}" invalid="[[invalid]]"
error-message="[[getErrorMessage_(invalid)]]"
invalid="[[invalid]]"
autofocus> autofocus>
</cr-input> </cr-input>
</div> </div>
......
...@@ -8,11 +8,10 @@ import 'chrome://resources/cr_elements/cr_input/cr_input.m.js'; ...@@ -8,11 +8,10 @@ import 'chrome://resources/cr_elements/cr_input/cr_input.m.js';
import 'chrome://resources/cr_elements/shared_style_css.m.js'; import 'chrome://resources/cr_elements/shared_style_css.m.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js'; import 'chrome://resources/cr_elements/shared_vars_css.m.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
Polymer({ Polymer({
is: 'viewer-password-screen', is: 'viewer-password-dialog',
_template: html`{__html_template__}`, _template: html`{__html_template__}`,
...@@ -20,18 +19,8 @@ Polymer({ ...@@ -20,18 +19,8 @@ Polymer({
invalid: Boolean, invalid: Boolean,
}, },
get active() {
return this.$.dialog.open;
},
show() {
this.$.dialog.showModal();
},
close() { close() {
if (this.active) {
this.$.dialog.close(); this.$.dialog.close();
}
}, },
deny() { deny() {
...@@ -51,14 +40,4 @@ Polymer({ ...@@ -51,14 +40,4 @@ Polymer({
this.$.submit.disabled = true; this.$.submit.disabled = true;
this.fire('password-submitted', {password: password.value}); this.fire('password-submitted', {password: password.value});
}, },
/**
* Returns |message| if input is invalid, otherwise empty string.
* This avoids setting the error message (which announces to screen readers)
* when there is no error.
* @return {string}
*/
getErrorMessage_() {
return this.invalid ? loadTimeData.getString('passwordInvalid') : '';
}
}); });
...@@ -221,9 +221,11 @@ ...@@ -221,9 +221,11 @@
<div id="sizer"></div> <div id="sizer"></div>
</template> </template>
<viewer-password-screen id="password-screen" <template is="dom-if" if="[[showPasswordDialog_]]" restamp>
<viewer-password-dialog id="password-dialog" on-close="onPasswordDialogClose_"
on-password-submitted="onPasswordSubmitted_"> on-password-submitted="onPasswordSubmitted_">
</viewer-password-screen> </viewer-password-dialog>
</template>
<template is="dom-if" if="[[showPropertiesDialog_]]" restamp> <template is="dom-if" if="[[showPropertiesDialog_]]" restamp>
<viewer-properties-dialog id="properties-dialog" <viewer-properties-dialog id="properties-dialog"
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import './elements/viewer-error-screen.js'; import './elements/viewer-error-screen.js';
import './elements/viewer-password-screen.js'; import './elements/viewer-password-dialog.js';
import './elements/viewer-pdf-toolbar.js'; import './elements/viewer-pdf-toolbar.js';
import './elements/viewer-properties-dialog.js'; import './elements/viewer-properties-dialog.js';
import './elements/viewer-zoom-toolbar.js'; import './elements/viewer-zoom-toolbar.js';
...@@ -253,6 +253,12 @@ export class PDFViewerElement extends PDFViewerBaseElement { ...@@ -253,6 +253,12 @@ export class PDFViewerElement extends PDFViewerBaseElement {
value: false, value: false,
}, },
/** @private */
showPasswordDialog_: {
type: Boolean,
value: false,
},
/** @private */ /** @private */
showPropertiesDialog_: { showPropertiesDialog_: {
type: Boolean, type: Boolean,
...@@ -810,11 +816,7 @@ export class PDFViewerElement extends PDFViewerBaseElement { ...@@ -810,11 +816,7 @@ export class PDFViewerElement extends PDFViewerBaseElement {
setLoadState(loadState) { setLoadState(loadState) {
super.setLoadState(loadState); super.setLoadState(loadState);
if (loadState === LoadState.FAILED) { if (loadState === LoadState.FAILED) {
const passwordScreen = this.$$('#password-screen'); this.closePasswordDialog_();
if (passwordScreen && passwordScreen.active) {
passwordScreen.deny();
passwordScreen.close();
}
} }
} }
...@@ -829,9 +831,22 @@ export class PDFViewerElement extends PDFViewerBaseElement { ...@@ -829,9 +831,22 @@ export class PDFViewerElement extends PDFViewerBaseElement {
} }
} }
/** @private */
closePasswordDialog_() {
const passwordDialog = this.shadowRoot.querySelector('#password-dialog');
if (passwordDialog) {
passwordDialog.close();
}
}
/** @private */
onPasswordDialogClose_() {
this.showPasswordDialog_ = false;
}
/** /**
* An event handler for handling password-submitted events. These are fired * An event handler for handling password-submitted events. These are fired
* when an event is entered into the password screen. * when an event is entered into the password dialog.
* @param {!CustomEvent<{password: string}>} event a password-submitted event. * @param {!CustomEvent<{password: string}>} event a password-submitted event.
* @private * @private
*/ */
...@@ -997,12 +1012,10 @@ export class PDFViewerElement extends PDFViewerBaseElement { ...@@ -997,12 +1012,10 @@ export class PDFViewerElement extends PDFViewerBaseElement {
/** @override */ /** @override */
setDocumentDimensions(documentDimensions) { setDocumentDimensions(documentDimensions) {
super.setDocumentDimensions(documentDimensions); super.setDocumentDimensions(documentDimensions);
// If we received the document dimensions, the password was good so we
// can dismiss the password screen. // If the document dimensions are received, the password was correct and the
const passwordScreen = this.$$('#password-screen'); // password dialog can be dismissed.
if (passwordScreen && passwordScreen.active) { this.closePasswordDialog_();
passwordScreen.close();
}
if (this.toolbarEnabled_) { if (this.toolbarEnabled_) {
this.docLength_ = this.documentDimensions.pageDimensions.length; this.docLength_ = this.documentDimensions.pageDimensions.length;
...@@ -1023,15 +1036,14 @@ export class PDFViewerElement extends PDFViewerBaseElement { ...@@ -1023,15 +1036,14 @@ export class PDFViewerElement extends PDFViewerBaseElement {
* @private * @private
*/ */
handlePasswordRequest_() { handlePasswordRequest_() {
// If the password screen isn't up, put it up. Otherwise we're // Show the password dialog if it is not already shown. Otherwise, respond
// responding to an incorrect password so deny it. // to an incorrect password.
const passwordScreen = this.$$('#password-screen'); if (!this.showPasswordDialog_) {
assert(passwordScreen); this.showPasswordDialog_ = true;
if (!passwordScreen.active) {
this.hadPassword_ = true;
passwordScreen.show();
} else { } else {
passwordScreen.deny(); const passwordDialog = this.shadowRoot.querySelector('#password-dialog');
assert(passwordDialog);
passwordDialog.deny();
} }
} }
......
...@@ -13,7 +13,7 @@ const tests = [ ...@@ -13,7 +13,7 @@ const tests = [
function testHasElements() { function testHasElements() {
const viewer = /** @type {!PDFViewerElement} */ ( const viewer = /** @type {!PDFViewerElement} */ (
document.body.querySelector('pdf-viewer')); document.body.querySelector('pdf-viewer'));
const commonElements = ['viewer-password-screen', 'viewer-error-screen']; const commonElements = ['viewer-error-screen'];
const elementNames = const elementNames =
document.documentElement.hasAttribute('pdf-viewer-update-enabled') ? document.documentElement.hasAttribute('pdf-viewer-update-enabled') ?
['viewer-pdf-toolbar-new', 'viewer-pdf-sidenav', ...commonElements] : ['viewer-pdf-toolbar-new', 'viewer-pdf-sidenav', ...commonElements] :
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment