Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanity-check that options.maxCount is an int before "adding" it. #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GarrettAlbright
Copy link

Try this:

$(element).simplyCountable({'maxCount': '123', 'countDirection': 'up'});

The character count will look like "-123,123" due to the arithmetic in the reverseCount function and JavaScript's usage of the + operator to append strings:

    var reverseCount = function(ct){
      return ct - (ct*2) + options.maxCount;
    }

That's slightly amusing breakage. Whenever JS sees a string on one side of a + operator, it seems to convert the other side to a string and then append them.

Why am I passing a string in the maxCount param? Because I have a data-softlimit parameter on the input fields which I'm using to get the character limit, which in this case is more of a "suggested" limit than a hard one, from the back end to the front end. So my code looks something like:

    var limit = $e.attr('data-softlimit'),
      $counter = $('<div class="softlimit-counter">')
        .attr('id', counterId)
        .html(Drupal.t('<span class="count"></span>/@limit characters (recommendation)', {'@limit': limit}));
    $e.after($counter);
    $e.simplyCountable({'counter': '#' + counterId + ' .count', 'maxCount': limit, 'countDirection': 'up'})

…But a similar issue could probably happen if I were doing something more common like getting the limit from $e.attr('maxlength').

I could make the int on my side, but why not idiot-proof it in the library itself? It's just a one-line fix; I don't think we need to parseInt(options.maxCount) elsewhere because in most other places, we're subtracting with this value, not adding it - or casting it to a string anyway.

…t is a string, the whole thing becomes a string and options.maxCount is appended to the end (because JavaScript).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant