Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.3, 2.4.1, 2.5
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Files API
    • Labels:
    • Testing Instructions:
      Hide

      1. Turn editing on for a course.
      2. Drag a .jar file into a course week.
      3. Update settings for the .jar file and ensure that the 'Show size' and 'Show type' options are turned on, then save and return to course.

      EXPECTED:

      The size and type display next to the jar file is similar to: "29.8MB application/java-archive"

      BEFORE THIS FIX:

      Display is similar to: "29.8MB application/zip"

      Show
      1. Turn editing on for a course. 2. Drag a .jar file into a course week. 3. Update settings for the .jar file and ensure that the 'Show size' and 'Show type' options are turned on, then save and return to course. EXPECTED: The size and type display next to the jar file is similar to: "29.8MB application/java-archive" BEFORE THIS FIX: Display is similar to: "29.8MB application/zip"
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-37395-m24
    • Pull Master Branch:
      MDL-37395-master
    • Rank:
      47018

      Description

      In Moodle 2.2, if you uploaded a Java .jar application/library file it was identified as an unknown file type and given the generic file icon. This behaviour was acceptable.

      In Moodle 2.3, this behaviour has changed and Moodle now identifies the jar file as a 'Zip file' with appropriate icon. This can be confusing in cases where we are expecting students to download and run a Java application, but Moodle says it's a zip file. (Jar files are indeed a variant of zip file, but so are lots of things, and it's not very helpful. Kind of like saying that a Word document using one of the xml-based formats is a plain text file - it technically is, but...)

      Behaviour is inconsistent as a result of this change: jar files that were added before the update still have the old icon and no description, whereas new or modified files get the zip icon and description.

      The change is caused by MDL-33144, which makes Moodle use something equivalent to the 'file' command-line application to guess the file type based on its content, when the file extension is not one Moodle knows about.

      I think the best solution is probably to add jar files to the list of types that Moodle actually knows about. Will do a patch for this.

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          I have now coded this. Some notes:

          MIME type used is application/java-archive. This does not appear to have been officially registered but is the general consensus. (See http://en.wikipedia.org/wiki/JAR_%28file_format%29)

          Description is 'Java program component'. I wanted something short and user-friendly but which would allow for the fact that .jar files can represent an entire Java program, or a Java library, or a Java applet (all of which are things you might conceivably provide for download, for example on a programming course). This is the best I could come up with.

          Since this behaviour happened from 2.3, I propose that this should be applied to 2.3 stable branch and up, although maybe somebody has a different idea! Anyway, submitting for review.

          Show
          Sam Marshall added a comment - I have now coded this. Some notes: MIME type used is application/java-archive. This does not appear to have been officially registered but is the general consensus. (See http://en.wikipedia.org/wiki/JAR_%28file_format%29 ) Description is 'Java program component'. I wanted something short and user-friendly but which would allow for the fact that .jar files can represent an entire Java program, or a Java library, or a Java applet (all of which are things you might conceivably provide for download, for example on a programming course). This is the best I could come up with. Since this behaviour happened from 2.3, I propose that this should be applied to 2.3 stable branch and up, although maybe somebody has a different idea! Anyway, submitting for review.
          Hide
          Sam Marshall added a comment -

          Requesting peer review. In case changes are necessary, I've only done the master branch - will do 2.3 and 2.4 branches before submitting for integration.

          Show
          Sam Marshall added a comment - Requesting peer review. In case changes are necessary, I've only done the master branch - will do 2.3 and 2.4 branches before submitting for integration.
          Hide
          Dan Poltawski added a comment -

          Hi Sam,

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [N] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [N] Documentation
          [Y] Git
          [Y] Sanity check

          Basically this looks fine but:

          1. I don't think the language string is necessary. In 2.3 we fixed it so that the language string is just used if it helps to describe the content better than the mimetype itself. I don't think that 'Java program component' is particularly more understandable to a normal person than 'application/java-archive' so you can legitimately leave this off. (We had a discussion about this because I thought it was crazy to get translators to translate all mime-types).
          2. Your comment explanation of the addition of the jar type in the list is probably overkill. There are a lot of 'non-standard' types in the list like that, the list would get unwieldy if we explained them all. Of course i'm not one to advocate removing comments, but in this case I don't think its necessary.

          Happy for you to submit it for integration and you can take those comments or leave them

          Show
          Dan Poltawski added a comment - Hi Sam, [Y] Syntax [-] Output [Y] Whitespace [N] Language [-] Databases [Y] Testing [-] Security [N] Documentation [Y] Git [Y] Sanity check Basically this looks fine but: I don't think the language string is necessary. In 2.3 we fixed it so that the language string is just used if it helps to describe the content better than the mimetype itself. I don't think that 'Java program component' is particularly more understandable to a normal person than 'application/java-archive' so you can legitimately leave this off. (We had a discussion about this because I thought it was crazy to get translators to translate all mime-types). Your comment explanation of the addition of the jar type in the list is probably overkill. There are a lot of 'non-standard' types in the list like that, the list would get unwieldy if we explained them all. Of course i'm not one to advocate removing comments, but in this case I don't think its necessary. Happy for you to submit it for integration and you can take those comments or leave them
          Hide
          Sam Marshall added a comment -

          Thanks Dan! I have made both changes requested - deleted the comment (fine) and removed the lang string. Regarding the lang string thing, I disagree that displaying a MIME type is in any way suitable for students, but I can also understand that translating every type is unhelpful. Anyway that is a separate issue (with a workaround, i.e. we will have to advise people to turn off the 'display type' option in those cases) so since a decision was already made, I'll stick with it.

          On the plus side, that makes this a one-line change.

          I've made the other branches and will submit.

          Show
          Sam Marshall added a comment - Thanks Dan! I have made both changes requested - deleted the comment (fine) and removed the lang string. Regarding the lang string thing, I disagree that displaying a MIME type is in any way suitable for students, but I can also understand that translating every type is unhelpful. Anyway that is a separate issue (with a workaround, i.e. we will have to advise people to turn off the 'display type' option in those cases) so since a decision was already made, I'll stick with it. On the plus side, that makes this a one-line change. I've made the other branches and will submit.
          Hide
          Dan Poltawski added a comment -

          Hi Sam

          Regarding the lang string thing, I disagree that displaying a MIME type is in any way suitable for students, but I can also understand that translating every type is unhelpful.

          I wasn't really suggesting that we should do that as a president (this is why we have jpeg/word docs etc translateD), but in the case of something as 'obscure' as jar, I don't think it is especially helpful.

          Show
          Dan Poltawski added a comment - Hi Sam Regarding the lang string thing, I disagree that displaying a MIME type is in any way suitable for students, but I can also understand that translating every type is unhelpful. I wasn't really suggesting that we should do that as a president (this is why we have jpeg/word docs etc translateD), but in the case of something as 'obscure' as jar, I don't think it is especially helpful.
          Hide
          Sam Marshall added a comment -

          I agree the extra work for translators may not be justified for something as uncommon as jar files, I just think it would be preferable to not display a type at all vs. displaying a MIME type which will probably frighten students and tutors. But that's a separate issue not related to just adding the jar file type.

          Show
          Sam Marshall added a comment - I agree the extra work for translators may not be justified for something as uncommon as jar files, I just think it would be preferable to not display a type at all vs. displaying a MIME type which will probably frighten students and tutors. But that's a separate issue not related to just adding the jar file type.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
          Hide
          Andrew Davis added a comment -

          Works as described. Passing.

          Show
          Andrew Davis added a comment - Works as described. Passing.
          Hide
          Dan Poltawski added a comment -

          Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          Show
          Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: