Skip to content

Commit

Permalink
User import, fix ldap import closes #3012
Browse files Browse the repository at this point in the history
  • Loading branch information
@AdSchellevis
AdSchellevis committed Dec 4, 2018
1 parent 3c3e27a commit e17dc86
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/www/system_usermanager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ function get_user_privdesc(& $user)
});

// import ldap users
$("#import_ldap_users").click(function(){
$("#import_ldap_users").click(function(event){
event.preventDefault();
const url="system_usermanager_import_ldap.php";
var oWin = window.open(url,"OPNsense","width=620,height=400,top=150,left=150,scrollbars=yes");
Expand Down

8 comments on commit e17dc86

@fichtner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

odd, this works fine in Chrome oO

@AdSchellevis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even more funny, it used to work in Firefox too :)

@fichtner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a code audit then -.-

@AdSchellevis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't expect this happens a lot, it's actually wrong, but I can take a quick look around.

@fichtner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we maybe fix all click function prototypes? most have (event), but some have not. at least I know one in unbound where I introduced this problem too

@AdSchellevis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, event can also be called e, which is valid too.... I'll check event.preventDefault(); one moment.

@AdSchellevis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest looks ok.

@fichtner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, lesson learned... don't trust browsers...

Please sign in to comment.