public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aditya Toshniwal <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][RM4310] User can not connect to the Master Password with Enter button
Date: Mon, 10 Jun 2019 15:05:14 +0530
Message-ID: <CAM9w-_mStAKAox2OeezDYXP2953mFdkQYJK6c+3y7CmpUkPyyg@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozgD-eUgFDKwkMjuUEt3v25PsEDpMX+Kkp2qKsSctzs2A@mail.gmail.com>
References: <CAM9w-_mhLAoL3ZgY3w8SCH6WbSF=_11tU3Ueh-voAWkfgVZtVw@mail.gmail.com>
<CA+OCxowteeh7yjQLToCGWPtFsqSKK10E3wX6o8k8i5Rub-DoGA@mail.gmail.com>
<CAM9w-_mn7mBrAcpBqMayPPU-btXenfX5+YKmsp_8w5qFd0903w@mail.gmail.com>
<CA+OCxozgD-eUgFDKwkMjuUEt3v25PsEDpMX+Kkp2qKsSctzs2A@mail.gmail.com>
Hi,
On Mon, Jun 10, 2019 at 2:34 PM Dave Page <[email protected]> wrote:
> Hi
>
> On Wed, Jun 5, 2019 at 1:39 PM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hi,
>>
>> On Wed, Jun 5, 2019 at 4:48 PM Dave Page <[email protected]> wrote:
>>
>>> Hi
>>>
>>> On Wed, Jun 5, 2019 at 8:14 AM Aditya Toshniwal <
>>> [email protected]> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Attached is the patch to fix an issue where using browser autofills in
>>>> alertify dialogs triggers the help page button (#4317).
>>>> A workaround was added to master password dialog to avoid this, but
>>>> that in turn disabled the 'OK' button tigger on pressing enter button. This
>>>> is also fixed with this patch.(#4310)
>>>>
>>>
>>> This isn't quite right. When the browser auto-fills the password, the OK
>>> button remains disabled. I can hit enter though - however, I shouldn't be
>>> able to do that when the OK button is disabled (being the default button on
>>> the dialogue, it should respond to enter, unless another control which also
>>> accepts enter has focus (e.g. a multiline text area).
>>>
>> The autofilled password appears as set, but it is actually not set to
>> textbox unless you enter/click in the dropdown. We get the DOM element
>> value as blank even if it appears filled. This is how chromium autofill
>> behaves. Upon hitting enter the autofill value is set to the textbox and OK
>> button is enabled.
>>
>>>
>>> The OK button should be enabled - probably all the time, as the user
>>> could have entered a blank password, and most systems won't prevent a user
>>> from trying to use such a password.
>>>
>> I have made the changes to allow blank master password, plus enabling OK
>> button always. Attached is the updated patch.
>>
>
> I think you've slightly mis-understood what I was trying to say.
>
> - We should not allow a blank password.
> - We should not disable the OK button at all.
>
> My analogy was focussed on the fact that most systems never disable OK
> buttons on login dialogues, as some of those systems (but not all of
> course) may allow blank passwords.
>
Got it !! Attached is the updated patch.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Attachments:
[application/octet-stream] RM4310_4317_v3.patch (3.6K, 3-RM4310_4317_v3.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 4af69603..29c11c2e 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -12,7 +12,7 @@ define('pgadmin.browser', [
'sources/gettext', 'sources/url_for', 'require', 'jquery', 'underscore', 'underscore.string',
'bootstrap', 'sources/pgadmin', 'pgadmin.alertifyjs', 'bundled_codemirror',
'sources/check_node_visibility', './toolbar', 'pgadmin.help',
- 'sources/csrf', 'sources/keyboard_shortcuts', 'pgadmin.browser.utils',
+ 'sources/csrf', 'pgadmin.browser.utils',
'wcdocker', 'jquery.contextmenu', 'jquery.aciplugin', 'jquery.acitree',
'pgadmin.browser.preferences', 'pgadmin.browser.messages',
'pgadmin.browser.menu', 'pgadmin.browser.panel', 'pgadmin.browser.layout',
@@ -24,7 +24,7 @@ define('pgadmin.browser', [
tree,
gettext, url_for, require, $, _, S,
Bootstrap, pgAdmin, Alertify, codemirror,
- checkNodeVisibility, toolBar, help, csrfToken, keyboardFunc
+ checkNodeVisibility, toolBar, help, csrfToken
) {
window.jQuery = window.$ = $;
// Some scripts do export their object in the window only.
@@ -518,6 +518,9 @@ define('pgadmin.browser', [
this.message = message;
this.reset = reset;
},
+ build: function() {
+ Alertify.pgDialogBuild.apply(this);
+ },
setup:function() {
return {
buttons:[{
@@ -547,30 +550,13 @@ define('pgadmin.browser', [
},
prepare:function() {
let self = this;
- let $password = null;
- let $okBtn = $(self.__internal.buttons[3].element);
-
self.setContent(self.message);
- $password = $(self.elements.body).find('#password');
-
/* Reset button hide */
if(!self.reset) {
$(self.__internal.buttons[1].element).addClass('d-none');
} else {
$(self.__internal.buttons[1].element).removeClass('d-none');
}
-
- /* Enable ok only if password entered */
- $okBtn.prop('disabled', true);
- $password.on('input change keyup', (event)=>{
- keyboardFunc._stopEventPropagation(event);
-
- if($password.val() != '') {
- $okBtn.prop('disabled', false);
- } else {
- $okBtn.prop('disabled', true);
- }
- });
},
callback: function(event) {
let parentDialog = this;
diff --git a/web/pgadmin/static/js/alertify.pgadmin.defaults.js b/web/pgadmin/static/js/alertify.pgadmin.defaults.js
index 8150399b..592c1e56 100644
--- a/web/pgadmin/static/js/alertify.pgadmin.defaults.js
+++ b/web/pgadmin/static/js/alertify.pgadmin.defaults.js
@@ -266,6 +266,17 @@ define([
this.set('onresized', alertifyDialogResized.bind(this, true));
this.set('onmaximized', alertifyDialogResized);
this.set('onrestored', alertifyDialogResized);
+
+ /* Set the key to null if it is not defined
+ * When Browser autofill drop down value is clicked it raises a keyup event
+ * with undefined keyCode. The undefined keyCode matches the undefined key
+ * of alertify and triggers the button
+ */
+ for(let i=0; i<this.__internal.buttons.length; i++) {
+ if(_.isUndefined(this.__internal.buttons[i]['key'])) {
+ this.__internal.buttons[i]['key'] = null;
+ }
+ }
};
alertify.pgHandleItemError = function(xhr, error, message, args) {
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin][RM4310] User can not connect to the Master Password with Enter button
In-Reply-To: <CAM9w-_mStAKAox2OeezDYXP2953mFdkQYJK6c+3y7CmpUkPyyg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox