HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Press any login button on any site

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pressloginanysitebutton

Problem

I'm working on a script that will be able to press the login button on any site for an app I'm working on. I have it working (still a few edge cases to work out such as multiple submit buttons and iFrames and I'm going to rewrite it to make it prettier once I figure them out).

Can you think of any ways I could improve it or make it more robust?

Some notes:

  • form.submit() doesn't work consistently since many sites don't allow it to be executed via a script.



  • Three types of login buttons I see consistently are 'a', 'button', and 'input'.



```
var buttonElements = document.getElementsByTagName('button');
var inputElements = document.getElementsByTagName('input');
var linkElements = document.getElementsByTagName('a');
var loginElement = null;
// For Button/Input
if (buttonElements != null){
loginElement = getSubmitElement(buttonElements);
}
if (loginElement != null){
console.log(loginElement);
loginElement.click();
}
else{
if (inputElements != null){
loginElement = getSubmitElement(inputElements);
}
if (loginElement != null){
console.log(loginElement);
loginElement.click();
}
else{
if (linkElements != null){
loginElement = getLinkElement(linkElements);
}
if (loginElement != null){
console.log(loginElement);
loginElement.click();
}
}
}
// Used for link elements ()
function getLinkElement(elements){
for (i=0;i < elements.length; i++){
// Is a submit button
// Check ID class
if (isLoginElement(elements[i],'id')){
return elements[i];
}
}
for (i=0;i < elements.length; i++) {
if(isLoginElement(elements[i], 'name')){
return elements[i];
}
else if(isLoginElement(elements[i],'class')){
return elements[i];
}
}
return null;
}

// Use for Button or Input - Checks the id/name/class attributes
function getSubmitElement(elements){

Solution

I have a couple of thoughts on this in terms of coding style,
but not really regarding robustness.
I hope you'll get more in-depth reviews in terms of robustness too!

Simplifying the complex if-else chain of loginElement

In this code:

var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
}
if (loginElement != null){
    console.log(loginElement);
    loginElement.click();
}
else{
    // ...
}


Notice that if buttonElements is null, then loginElement will still be null after the first if statement, in which case the second if will be pointless.
This is equivalent to the above but without the wasted step:

var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
    if (loginElement != null){
        console.log(loginElement);
        loginElement.click();
    }
}
else{
    // ...
}


On closer look, the entire if-else chain can be simplified to this:

var loginElement = null;

if (buttonElements != null) {
    loginElement = getSubmitElement(buttonElements);
} else if (inputElements != null) {
    loginElement = getSubmitElement(inputElements);
} else if (linkElements != null) {
    loginElement = getLinkElement(linkElements);
}

if (loginElement != null) {
    console.log(loginElement);
    loginElement.click();
}


Cache duplicated operations in a local variable

In this statement,
in the worst case,
the type variable will be lowercased twice:

if (type.toLowerCase() == 'submit' || type.toLowerCase() == 'image'){


It would be better to lowercase only once and store in a local variable:

var normalizedType = type.toLowerCase();
if (normalizedType == 'submit' || normalizedType == 'image') {


Simplify else-if chains using early returns

When you return in the if-else-if branches like this:

if (isLoginElement(elements[i],'id')){
    return elements[i];
}
else if(isLoginElement(elements[i], 'name')){
    return elements[i];
}
else if(isLoginElement(elements[i],'class')){
    return elements[i];
}


You can drop the else, which makes it slightly simpler:

if (isLoginElement(elements[i],'id')) {
    return elements[i];
}
if (isLoginElement(elements[i], 'name')) {
    return elements[i];
}
if (isLoginElement(elements[i],'class')) {
    return elements[i];
}


Other simplification

This code:

if (attr != null){
    if (hasLogin(attr)){
        return true;
    }
}
return false;


Can be written simpler as:

return attr != null && hasLogin(attr);

Code Snippets

var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
}
if (loginElement != null){
    console.log(loginElement);
    loginElement.click();
}
else{
    // ...
}
var loginElement = null;
if (buttonElements != null){
    loginElement = getSubmitElement(buttonElements);
    if (loginElement != null){
        console.log(loginElement);
        loginElement.click();
    }
}
else{
    // ...
}
var loginElement = null;

if (buttonElements != null) {
    loginElement = getSubmitElement(buttonElements);
} else if (inputElements != null) {
    loginElement = getSubmitElement(inputElements);
} else if (linkElements != null) {
    loginElement = getLinkElement(linkElements);
}

if (loginElement != null) {
    console.log(loginElement);
    loginElement.click();
}
if (type.toLowerCase() == 'submit' || type.toLowerCase() == 'image'){
var normalizedType = type.toLowerCase();
if (normalizedType == 'submit' || normalizedType == 'image') {

Context

StackExchange Code Review Q#79268, answer score: 2

Revisions (0)

No revisions yet.