patternjavascriptMinor
Press any login button on any site
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:
```
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){
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
In this code:
Notice that if
This is equivalent to the above but without the wasted step:
On closer look, the entire
Cache duplicated operations in a local variable
In this statement,
in the worst case,
the
It would be better to lowercase only once and store in a local variable:
Simplify
When you return in the
You can drop the
Other simplification
This code:
Can be written simpler as:
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 loginElementIn 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 returnsWhen 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.