From 12914307f1476bb10f896dab0c6cf8cb3055c4fc Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Mon, 3 Jul 2017 11:52:12 -0700 Subject: [PATCH] Improve experience when discovering identity key error on send New experience in the Message Detail view when outgoing identity key errors happen, matching the Android View. 'View' button is only shown on outgoing key errors right now. When a contact with an outgoing identity key error is clicked, they are taken to a view like the popup that comes up on Android: an explanation of what happened and three options: 'Show Safety Number', 'Send Anyway', and 'Cancel' Contacts are now sorted alphabetically, with the set of contacts with errors coming before the rest. FREEBIE --- _locales/en/messages.json | 16 ++- background.html | 37 +++++-- js/models/conversations.js | 8 ++ js/views/conversation_view.js | 21 +++- js/views/identity_key_send_error_view.js | 51 ++++++++++ js/views/message_detail_view.js | 122 +++++++++++++++-------- stylesheets/_conversation.scss | 38 ++++++- stylesheets/manifest.css | 28 +++++- test/index.html | 15 +++ 9 files changed, 271 insertions(+), 65 deletions(-) create mode 100644 js/views/identity_key_send_error_view.js diff --git a/_locales/en/messages.json b/_locales/en/messages.json index e52d15b23f..61a5424c61 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -3,6 +3,10 @@ "message": "Me", "description": "The label for yourself when shown in a group member list" }, + "view": { + "message": "View", + "description": "Used as a label on a button allowing user to see more information" + }, "youLeftTheGroup": { "message": "You left the group", "description": "Displayed when a user can't send a message because they have left the group" @@ -105,9 +109,15 @@ } } }, - "retryDescription": { - "message": "You can retry sending this message to each of the failed recipients with these buttons:", - "description": "Shows on the message details view when it's a message error which can be retried." + "identityKeyErrorOnSend": { + "message": "Your safety number with $name$ has changed. This could either mean that someone is trying to intercept your communication or that $name$ has simply reinstalled Signal. You may wish to verify your saftey number with this contact.", + "description": "Shown when user clicks on a failed recipient in the message detail view after an identity key change", + "placeholders": { + "name": { + "content": "$1", + "example": "Bob" + } + } }, "sendAnyway": { "message": "Send Anyway", diff --git a/background.html b/background.html index fa26a92ef2..17bac7fbd6 100644 --- a/background.html +++ b/background.html @@ -297,14 +297,6 @@ + - - + + + diff --git a/js/models/conversations.js b/js/models/conversations.js index 9d9e56ab34..ffa667d6f9 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -202,6 +202,14 @@ }.bind(this))); } }, + setTrusted: function() { + if (!this.isPrivate()) { + throw new Error('You cannot set a group conversation as trusted. ' + + 'You must set individual contacts as trusted.'); + } + + return textsecure.storage.protocol.setApproval(this.id, true); + }, isUntrusted: function() { if (this.isPrivate()) { return textsecure.storage.protocol.isUntrusted(this.id); diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index daad70db77..f25dc94160 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -202,6 +202,13 @@ }.bind(this))); }, + + markAllAsApproved: function(untrusted) { + return Promise.all(untrusted.map(function(contact) { + return contact.setApproved(); + }.bind(this))); + }, + openSafetyNumberScreens: function(unverified) { if (unverified.length === 1) { this.showSafetyNumber(null, unverified.at(0)); @@ -620,7 +627,10 @@ messageDetail: function(e, data) { var view = new Whisper.MessageDetailView({ model: data.message, - conversation: this.model + conversation: this.model, + // we pass these in to allow nested panels + listenBack: this.listenBack.bind(this), + resetPanel: this.resetPanel.bind(this) }); this.listenBack(view); view.render(); @@ -749,10 +759,17 @@ _.defaults(options, {force: false}); this.model.getUntrusted().then(function(contacts) { - if (!contacts.length || options.force) { + + if (!contacts.length) { return this.sendMessage(e); } + if (options.force) { + return this.markAllAsApproved(contacts).then(function() { + this.sendMessage(e); + }.bind(this)); + } + this.showSendConfirmationDialog(e, contacts); }.bind(this)); }, diff --git a/js/views/identity_key_send_error_view.js b/js/views/identity_key_send_error_view.js new file mode 100644 index 0000000000..99c5d06666 --- /dev/null +++ b/js/views/identity_key_send_error_view.js @@ -0,0 +1,51 @@ +/* + * vim: ts=4:sw=4:expandtab + */ +(function () { + 'use strict'; + window.Whisper = window.Whisper || {}; + + Whisper.IdentityKeySendErrorPanelView = Whisper.View.extend({ + className: 'identity-key-send-error panel', + templateName: 'identity-key-send-error', + initialize: function(options) { + this.listenBack = options.listenBack; + this.resetPanel = options.resetPanel; + + this.wasUnverified = this.model.isUnverified(); + this.listenTo(this.model, 'change', this.render); + }, + events: { + 'click .show-safety-number': 'showSafetyNumber', + 'click .send-anyway': 'sendAnyway', + 'click .cancel': 'cancel' + }, + showSafetyNumber: function() { + var view = new Whisper.KeyVerificationPanelView({ + model: this.model + }); + this.listenBack(view); + }, + sendAnyway: function() { + this.resetPanel(); + this.trigger('send-anyway'); + }, + cancel: function() { + this.resetPanel(); + }, + render_attributes: function() { + var send = i18n('sendAnyway'); + if (this.wasUnverified && !this.model.isUnverified()) { + send = i18n('resend'); + } + + var errorExplanation = i18n('identityKeyErrorOnSend', this.model.getTitle(), this.model.getTitle()); + return { + errorExplanation : errorExplanation, + showSafetyNumber : i18n('showSafetyNumber'), + sendAnyway : send, + cancel : i18n('cancel') + }; + } + }); +})(); diff --git a/js/views/message_detail_view.js b/js/views/message_detail_view.js index b9de420da2..fccc19f307 100644 --- a/js/views/message_detail_view.js +++ b/js/views/message_detail_view.js @@ -9,18 +9,68 @@ className: 'contact-detail', templateName: 'contact-detail', initialize: function(options) { - this.errors = _.reject(options.errors, function(e) { - return (e.name === 'OutgoingIdentityKeyError' || - e.name === 'OutgoingMessageError' || - e.name === 'SendMessageNetworkError'); - }); + this.listenBack = options.listenBack; + this.resetPanel = options.resetPanel; + this.message = options.message; + var newIdentity = i18n('newIdentity'); + this.errors = _.map(options.errors, function(error) { + if (error.name === 'OutgoingIdentityKeyError') { + error.message = newIdentity; + } + return error; + }); + this.outgoingKeyError = _.find(this.errors, function(error) { + return error.name === 'OutgoingIdentityKeyError'; + }); + }, + events: { + 'click': 'onClick' + }, + onClick: function() { + if (this.outgoingKeyError) { + var view = new Whisper.IdentityKeySendErrorPanelView({ + model: this.model, + listenBack: this.listenBack, + resetPanel: this.resetPanel + }); + + this.listenTo(view, 'send-anyway', this.onSendAnyway); + + view.render(); + this.listenBack(view); + } + // TODO: is there anything we might want to do here? Pop a confirmation dialog? Ideally it would always have error-specific help. + }, + forceSend: function() { + this.model.updateVerified().then(function() { + if (this.model.isUnverified()) { + return this.model.setVerifiedDefault(); + } + }.bind(this)).then(function() { + return this.model.isUntrusted(); + }.bind(this)).then(function(untrusted) { + if (untrusted) { + return this.model.setTrusted(); + } + }.bind(this)).then(function() { + this.message.resend(this.outgoingKeyError.number); + }.bind(this)); + }, + onSendAnyway: function() { + if (this.outgoingKeyError) { + this.forceSend(); + } }, render_attributes: function() { + var showButton = Boolean(this.outgoingKeyError); + return { - name : this.model.getTitle(), - avatar : this.model.getAvatar(), - errors : this.errors + name : this.model.getTitle(), + avatar : this.model.getAvatar(), + errors : this.errors, + showErrorButton : showButton, + errorButtonLabel : i18n('view') }; } }); @@ -29,23 +79,15 @@ className: 'message-detail panel', templateName: 'message-detail', initialize: function(options) { + this.listenBack = options.listenBack; + this.resetPanel = options.resetPanel; + this.view = new Whisper.MessageView({model: this.model}); this.view.render(); this.conversation = options.conversation; this.listenTo(this.model, 'change', this.render); }, - events: { - 'click button.retry': 'onRetry' - }, - onRetry: function(e) { - var number = _.find(e.target.attributes, function(attribute) { - return attribute.name === 'data-number'; - }); - if (number) { - this.model.resend(number.value); - } - }, getContact: function(number) { var c = ConversationController.get(number); return { @@ -53,15 +95,6 @@ title: c ? c.getTitle() : number }; }, - buildRetryTargetList: function() { - var targets = _.filter(this.model.get('errors'), function(e) { - return e.number && e.name === 'OutgoingIdentityKeyError'; - }); - - return _.map(targets, function(e) { - return this.getContact(e.number); - }.bind(this)); - }, contacts: function() { if (this.model.isIncoming()) { var number = this.model.get('source'); @@ -71,25 +104,25 @@ } }, renderContact: function(contact) { - var grouped = _.groupBy(this.model.get('errors'), 'number'); - var view = new ContactView({ model: contact, - errors: grouped[contact.id] + errors: this.grouped[contact.id], + listenBack: this.listenBack, + resetPanel: this.resetPanel, + message: this.model }).render(); this.$('.contacts').append(view.el); }, render: function() { - var retryTargets = this.buildRetryTargetList(); - var allowRetry = retryTargets.length > 0; + var errorsWithoutNumber = _.reject(this.model.get('errors'), function(error) { + return Boolean(error.number); + }); this.$el.html(Mustache.render(_.result(this, 'template', ''), { sent_at : moment(this.model.get('sent_at')).format('LLLL'), received_at : this.model.isIncoming() ? moment(this.model.get('received_at')).format('LLLL') : null, tofrom : this.model.isIncoming() ? i18n('from') : i18n('to'), - errors : this.model.get('errors'), - allowRetry : allowRetry, - retryTargets : retryTargets, + errors : errorsWithoutNumber, title : i18n('messageDetail'), sent : i18n('sent'), received : i18n('received'), @@ -98,14 +131,21 @@ })); this.view.$el.prependTo(this.$('.message-container')); + this.grouped = _.groupBy(this.model.get('errors'), 'number'); if (this.model.isOutgoing()) { - this.conversation.contactCollection.reject(function(c) { + var contacts = this.conversation.contactCollection.reject(function(c) { return c.isMe(); - }).forEach(this.renderContact.bind(this)); + }); + + _.sortBy(contacts, function(c) { + var prefix = this.grouped[c.id] ? '0' : '1'; + // this prefix ensures that contacts with errors are listed first; + // otherwise it's alphabetical + return prefix + c.getTitle(); + }.bind(this)).forEach(this.renderContact.bind(this)); } else { - this.renderContact( - this.conversation.contactCollection.get(this.model.get('source')) - ); + var c = this.conversation.contactCollection.get(this.model.get('source')); + this.renderContact(c); } } }); diff --git a/stylesheets/_conversation.scss b/stylesheets/_conversation.scss index 96526899ab..4bc0593ac5 100644 --- a/stylesheets/_conversation.scss +++ b/stylesheets/_conversation.scss @@ -39,7 +39,7 @@ .container { padding-top: 20px; - max-width: 950px; + max-width: 750px; margin: 0 auto; padding: 20px; } @@ -77,10 +77,6 @@ } .key-verification { - .container { - max-width: 750px; - } - label { display: block; margin: 10px 0; @@ -150,6 +146,24 @@ } } +.identity-key-send-error { + button { + margin-top: 0px; + margin-bottom: 0px; + } + .explanation { + margin-top: 20px; + } + .safety-number { + margin-top: 30px; + text-align: center; + } + .actions { + margin-top: 30px; + text-align: center; + } +} + .message-detail { background-color: #eee; @@ -198,6 +212,20 @@ float: right; } + button.error { + background-color: red; + color: white; + + span.icon.error { + display: inline-block; + width: 1.25em; + height: 1.25em; + position: relative; + vertical-align: middle; + @include color-svg('/images/warning.svg', white); + } + } + .error-message { margin: 6px 0 0; font-size: $font-size-small; diff --git a/stylesheets/manifest.css b/stylesheets/manifest.css index ca684fa111..45842eb7ca 100644 --- a/stylesheets/manifest.css +++ b/stylesheets/manifest.css @@ -1057,7 +1057,7 @@ input.search { overflow-y: scroll; } .conversation .panel .container { padding-top: 20px; - max-width: 950px; + max-width: 750px; margin: 0 auto; padding: 20px; } .conversation .main.panel { @@ -1083,8 +1083,6 @@ input.search { .discussion-container { background-color: #eee; } -.key-verification .container { - max-width: 750px; } .key-verification label { display: block; margin: 10px 0; @@ -1139,6 +1137,18 @@ input.search { padding: 10px; margin: 0; } +.identity-key-send-error button { + margin-top: 0px; + margin-bottom: 0px; } +.identity-key-send-error .explanation { + margin-top: 20px; } +.identity-key-send-error .safety-number { + margin-top: 30px; + text-align: center; } +.identity-key-send-error .actions { + margin-top: 30px; + text-align: center; } + .message-detail { background-color: #eee; } .message-detail .message-container { @@ -1168,6 +1178,18 @@ input.search { margin-bottom: 5px; } .message-detail .contacts .contact-detail .error-icon-container { float: right; } + .message-detail .contacts .contact-detail button.error { + background-color: red; + color: white; } + .message-detail .contacts .contact-detail button.error span.icon.error { + display: inline-block; + width: 1.25em; + height: 1.25em; + position: relative; + vertical-align: middle; + -webkit-mask: url("/images/warning.svg") no-repeat center; + -webkit-mask-size: 100%; + background-color: white; } .message-detail .contacts .contact-detail .error-message { margin: 6px 0 0; font-size: 0.92857em; diff --git a/test/index.html b/test/index.html index 17b9f724b3..676bd198db 100644 --- a/test/index.html +++ b/test/index.html @@ -316,6 +316,20 @@ + +