Change Javascript function body with jstl core tag. WTF or no?



  • So I was looking through the source code and found this.

    function golink(x){
    		<c:if test="${empty userSession}" >
    			alert("You need to log in.");
    			return ;
    		</c:if>
    		location.href=x;
    	}
    

    This is used to check if a user has access to a menu.

    Is this a WTF?


  • Notification Spam Recipient

    Maybe?



  • Is that coldfusion emitting JavaScript? With a bonus of server-side authentication that is also client-side-only?



  • @ben_lubar said:

    Is that coldfusion emitting JavaScript? With a bonus of server-side authentication that is also client-side-only?

    No. The javascript function is just written in a jsp file.
    They don't use Coldfusion.

    ${empty userSession}
    

    Here, userSession is just a request attribute bound when a user successfully logs in.



  • I don't know what a "jstl core tag" is, so I can't really comment.

    But considering the other stuff you've posted, I have to say WTF.



  • Well, basically the snipped above manipulates the function body with the server side script.
    So if you have a certain object bound to your session, the body of that javascript function changes.

    I'm not sure it's a good thing.



  • It's a WTF.

    Sometimes you need to get backend variables into javascript. The proper way to do this is to separate all such code from the rest of your javascripts. Something like:

    <script type="javascript">
       window.GLOBALS = {
          loggedIn: <c:if ...>
       };
    </script>
    

    Then later you can have pure javascript access window.GLOBALS.loggedIn and make decisions based on that.

    Also, what's up with using just the alert? That's for debugging/testing. Create a real user interface, for God's sake.



  • Yeah that looks better. I still feel that mixing javascript and backend variables is icky.

    @cartman82 said:

    Also, what's up with using just the alert? That's for debugging/testing. Create a real user interface, for God's sake.

    Yeah, it's everywhere. Alerts and confirms. Every time a user posts a record, he gets an alert saying something like "Post success" or whenever he deletes something, then "Failed to delete" in case of a failure.

    All alerts.

    It's an 8 million dollar project, baby 😉



  • @Ascendant said:

    Yeah that looks better. I still feel that mixing javascript and backend variables is icky.

    The "pure" way to do it would be with some kind of JSON script tag (<script type="application/json" src="/api/config">), which you then parse during the page init.

    But that would add an additional request to the page load. The first solution is the best tradeoff between performance and purity that I know.



  • @Ascendant said:

    Yeah, it's everywhere. Alerts and confirms. Every time a user posts a record, he gets an alert saying something like "Post success" or whenever he deletes something, then "Failed to delete" in case of a failure.

    We used to have a thing where almost every action went through a confirm. So things like "Post record." (Literally with the period at the end, no question mark) even when "post" or "record" don't make that much sense to the end-user. It was determined that we should get rid of these confirms. So instead we have a jQueryUI modal with text "Posting record" in the case it can take a few seconds to post it and do the processing. In most cases the text is better, like "posting record to third party" or something.


  • Discourse touched me in a no-no place

    @Ascendant said:

    This is used to check if a user has access to a menu.

    Is this a WTF?

    Why not

    1­. not display the link anyway
    b. rely on the check in the target href. There is a check on the target href isn't there...?


  • ♿ (Parody)

    Can you do something like disable the menu if userSession is empty? That would be handled when the jsp is processed. No extra requests, no javascript. Not knowing how the menu is implemented, this might or might not be viable.



  • It's a WTF because it breaks syntax javascript highlighting, linters, and minifiers.

    @cartman82 said:

    <script type="javascript">
    window.GLOBALS = {
    loggedIn: <c:if ...>
    };
    </script>

    @cartman82's solution is on the right track, but I would make a reusable component that generates the entire script block and handles stuff like quote escaping. The end result would look something like this:

    <c:out value="createScriptBlock(userSession)" escapeXml="false" />
    
    <script language="javascript">
    function golink(x) {
      if(!userSession) {
        alert("You need to log in.");
        return;
      }
      location.href=x;
    }
    </script>
    


  • @Jaime said:

    <c:out value="createScriptBlock(userSession)" escapeXml="false" />

    Agreed, this could work better. Depends on the templating language.

    The downside of this particular example is that your javascript intellisense won't like it very much. It'll freak out when it sees this userSession global, which isn't defined anywhere.

    If you can stuff server-side code into a javascript compatible format, that's the best of both worlds.

    Eg.

    <script>
       window.SETTINGS = {};
       window.SETTINGS.userSession = '<?php echo "yes" ?>' === 'yes';
    </script>
    

    Like I said, depends on the templating language.



  • Do you think the developers at his project are working with an IDE with syntax highlighting and intellisense?

    Based on what he's posted, I'd be surprised if they had learned to move the crate underneath the hanging banana yet.



  • @blakeyrat said:

    I don't know what a "jstl core tag" is, so I can't really comment.

    But considering the other stuff you've posted, I have to say WTF.

    I could explain it, but I doubt you'd care.

    Suffice it to say, it's something you'd use in late 1990s Java web development. You know, during the Classic ASP-style era.



  • Mostly, it's a WTF because the link protection is client-side. The user can defeat that easily if they know anything at all about javascript.



  • @anotherusername said:

    Mostly, it's a WTF because the link protection is client-side. The user can defeat that easily if they know anything at all about javascript.

    ...which could have been avoided using a <c:choose> / <c:when> / <c:otherwise> block.

    Filed under: I have no idea why there are no <c:else> or <c:elseif> tags


  • :belt_onion:

    I've personally done the following for some dynamic variables.

    var step=0;
    <c:if test={foo}>
    step=1;
    </c:if>
    

    Which, technically leads to funky generated source...

    var step=0;
    step=1;
    

    But generally works pretty well.

    But yeah, inserting an alert via <c:if> tags? WTF. Not a big one, but it's a WTF


  • Trolleybus Mechanic

    @Ascendant said:

    Is this a WTF?

    Without reading the post, and knowing your posting history-- yes. Yes it is.

    @Ascendant said:

    function golink

    This name is a wtf.

    @Ascendant said:

    alert("You need to log in.");

    Using an alert is a wtf.

    @Ascendant said:

    return ;

    Returning anything that isn't "false" on a click handler is a wtf.

    @Ascendant said:

    location.href=x;

    Using javascript redirect instead of letting the browser handle a hyperlink is a wtf.

    Also assuming there is no server-side validation on the user's logged-in status: that is also a wtf.


Log in to reply