Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-12699

Change theme builder JS compiler to use WHITESPACE_ONLY option

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.2
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-10426Theme builder requires copy to temp directory
      KULRICE-9905Document theme builder and updates to view theme
      KULRICE-10111Document ViewTheme and theme builder
      KULRICE-9874Work through warnings from closure compiler
      KULRICE-9904Add unit tests for theme builder
      KULRICE-10427Allow for less files to be used directly in dev mode
      KULRICE-9890Clean up images and change references to use theme image paths
      KULRICE-7357Handle naming of JS files to avoid problems caused by cached JS files
      KULRICE-13990Add github-pull-request-builder-plugin to jenkins
      KULRICE-9964The POM for org.kuali.rice:rice-krad-theme-builder:jar:2.3.0-SNAPSHOT is missing
    • Rice Module:
      KRAD
    • Application Requirement:
      KS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      Doing a bit of research, I see this in the docs for the closure compiler:
      “Compilation with SIMPLE_OPTIMIZATIONS always preserves the functionality of syntactically valid JavaScript, provided that the code does not access local variables using string names (by using eval() statements, for example)."

      So, I’d suggest either rethinking lots of the usages of eval all over the code, and/or switching to the WHITESPACE_ONLY option. Or, at least making that option configurable somewhere so that we can change it ourselves when building our own theme.

      Chris

      From: <Maurer>, Christopher Maurer <chmaurer@iu.edu>
      Date: Monday, May 19, 2014 at 3:13 PM
      To: Christopher Maurer <chmaurer@iu.edu>, "rice.usergroup.krad@kuali.org" <rice.usergroup.krad@kuali.org>
      Subject: Re:

      {Rice KRAD User Group} Possible problem in krad.request.js with the preSubmitCall

      I have a second case of this in https://jira.kuali.org/browse/KSAP-1337.
      Both cases are calling an eval() where the contents of that eval are referencing a js variable that has been renamed during the minification’s compiler optimizations.

      Chris

      From: <Maurer>, Christopher Maurer <chmaurer@iu.edu>
      Date: Monday, May 19, 2014 at 2:00 PM
      To: "rice.usergroup.krad@kuali.org" <rice.usergroup.krad@kuali.org>
      Subject: {Rice KRAD User Group}

      Possible problem in krad.request.js with the preSubmitCall

      I at first thought this might be due to the upgrade to 2.4, but I believe that it was an existing problem back when we were on 2.3.2 as well.
      But what I’m seeing is that when the minification is run on krad.request.js for the theme, it’s stripping out some important parts!
      On line 148, there’s a variable declaration, but it never gets used. It could be used depending on what’s in the preSubmitCall, but the js compiler doing the minification doesn’t know that. So I believe that it’s stripping it out. Then, when we try to call a function like this:
      return kradRequest.confirm('KS-Uif-Confirmation-Dialog’);

      We get a JS error that kradRequest is not defined.

      (Original jira is this: https://jira.kuali.org/browse/KSENROLL-9274 where this recommendation was made)

      Is there any good way around this on my end? Do we really trust the compiler to do appropriate cleanup? Should the theme builder switch from SIMPLE_OPTIMIZATIONS to WHITESPACE_ONLY to protect the js that was intentionally written?

      Chris

        Issue Links

          Activity

          Hide
          Kristina Taylor (Inactive) added a comment -

          We have decided to just switch over to the WHITESPACE_ONLY option for now.

          Show
          Kristina Taylor (Inactive) added a comment - We have decided to just switch over to the WHITESPACE_ONLY option for now.

            People

            • Assignee:
              Kristina Taylor (Inactive)
              Reporter:
              Jerry Neal (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1 hour
                1h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour
                1h

                  Structure Helper Panel