Moodle
  1. Moodle
  2. MDL-35622

Internet Explorer 9 problems with smartboard files.

    Details

    • Workaround:
      Hide

      Test pre-requisites

      Test steps

      1. Do not upgrade!
      2. Create a new folder
      3. Add each of the above files in there
      4. Upgrade Moodle
      5. Make sure you haven't encountered any error
      6. Create another folder
      7. Upload each files again
      8. Browse through the database and make sure all the files in both folders are identified as mimetype: application/x-smarttech-notebook
      9. Go to any of the folders
      10. Open Firebug net console (or equivalent)
      11. Download each of the files and make sure the response headers match the Content-Type application/x-smarttech-notebook
      12. Have a Kit Kat.

      Test 2

      1. In IE8, 9, open the folder with previously updated files
      2. Download each of them and make sure they're not renamed as .zip upon saving
      Show
      Test pre-requisites Do not upgrade yet! Install an extension to read the HTTP headers (For Firefox: https://addons.mozilla.org/en-US/firefox/addon/live-http-headers/ ) Download one sample of each of the following files .notebook .nbk .gallery .galleryitem .gallerycollection .xbk Test steps Do not upgrade! Create a new folder Add each of the above files in there Upgrade Moodle Make sure you haven't encountered any error Create another folder Upload each files again Browse through the database and make sure all the files in both folders are identified as mimetype : application/x-smarttech-notebook Go to any of the folders Open Firebug net console (or equivalent) Download each of the files and make sure the response headers match the Content-Type application/x-smarttech-notebook Have a Kit Kat. Test 2 In IE8, 9, open the folder with previously updated files Download each of them and make sure they're not renamed as .zip upon saving
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
    • Pull Master Branch:
      MDL-35622-master
    • Rank:
      44351

      Description

      Hello we have a problem when we upload a resource file to a course. When we upload the file it uploads fine but when you goto the page and download it the file extension on the file has been changed to .zip. The file is a smart notebook file. I have included an example here. We are on moodle version 2.3
      This seems to be some kind of a problem with Internet explorer as google chrome works fine. But we have over 600 PC's and installing google chrome on them all is not an option.

      1. 1More1Less.xbk
        1.23 MB
        Frédéric Massart
      2. Curriculum Map.xbk
        3 kB
        Chris Follin
      3. testnotebook.xbk
        5 kB
        David Broadley

        Issue Links

          Activity

          Hide
          Paul Holden added a comment -

          The problem is that Moodle is storing and sending an incorrect mimetype for notebook files, which causes Internet Explorer to flake out when trying to download them - I've created a branch on my github that adds this missing mimetype; https://github.com/paulholden/moodle/compare/master...MDL-35622

          I don't know if this will fix all the previously stored incorrect mimetypes for notebook files though...

          Show
          Paul Holden added a comment - The problem is that Moodle is storing and sending an incorrect mimetype for notebook files, which causes Internet Explorer to flake out when trying to download them - I've created a branch on my github that adds this missing mimetype; https://github.com/paulholden/moodle/compare/master...MDL-35622 I don't know if this will fix all the previously stored incorrect mimetypes for notebook files though...
          Hide
          Marina Glancy added a comment -

          thanks for the fix Paul,
          it won't change the mimetype for existing files. There should be an upgrade script for this

          Show
          Marina Glancy added a comment - thanks for the fix Paul, it won't change the mimetype for existing files. There should be an upgrade script for this
          Hide
          Paul Holden added a comment -

          Hi Marina,

          I've added another commit to the same branch that will set the correct mimetype for existing files during upgrade; https://github.com/paulholden/moodle/commit/3076cb791a6c1e5b3c73d8cb7b15947169a0b43b

          Show
          Paul Holden added a comment - Hi Marina, I've added another commit to the same branch that will set the correct mimetype for existing files during upgrade; https://github.com/paulholden/moodle/commit/3076cb791a6c1e5b3c73d8cb7b15947169a0b43b
          Hide
          Matteo Scaramuccia added a comment -

          Why not adding all the SMART Board stuff e.g. http://moodle.org/mod/forum/discuss.php?d=211879#p925127?
          Something like:

          https://github.com/scara/moodle/commit/4bbc04f9f393e0236b89cd76ffe20df01b83efb6

          I'm not sure about the version bumping, I guess it is correct to just ++1 on the incremental part of the current $version.

          Show
          Matteo Scaramuccia added a comment - Why not adding all the SMART Board stuff e.g. http://moodle.org/mod/forum/discuss.php?d=211879#p925127? Something like: https://github.com/scara/moodle/commit/4bbc04f9f393e0236b89cd76ffe20df01b83efb6 I'm not sure about the version bumping, I guess it is correct to just ++1 on the incremental part of the current $version .
          Hide
          David Broadley added a comment -

          Thanks for all these. Altering mime types works fine

          Show
          David Broadley added a comment - Thanks for all these. Altering mime types works fine
          Hide
          Frédéric Massart added a comment -

          Thank you for your feedbacks and patches. I have tried to replicate this issue on IE9 and IE8 using Windows 7 but I couldn't. I made sure the setting "Open file based on their content" was set to True in IE8, MIME Sniffing enable on IE9. Every time I tried to download the .notebook file it would been saved correctly.

          Can I ask you to provide with replications, perhaps settings of IE, and a sample of each file present in Matteo's patch? By the way, the patch is good as it is, but we need to be able to test it.

          Many thanks!
          Fred

          Show
          Frédéric Massart added a comment - Thank you for your feedbacks and patches. I have tried to replicate this issue on IE9 and IE8 using Windows 7 but I couldn't. I made sure the setting "Open file based on their content" was set to True in IE8, MIME Sniffing enable on IE9. Every time I tried to download the .notebook file it would been saved correctly. Can I ask you to provide with replications, perhaps settings of IE, and a sample of each file present in Matteo's patch? By the way, the patch is good as it is, but we need to be able to test it. Many thanks! Fred
          Hide
          Paul Holden added a comment -

          Frédéric, upload some files with .notebook extension and confirm the mimetype is being stored and sent incorrectly - my patch should fix these on upgrade.

          After applying the patch, upload another file with .notebook extension and confirm that the correct mimetype is stored in the 'files' table and sent as a header when downloading the file (the Live HTTP Headers Firefox plugin is useful for this). We had numerous reports from members of staff here regarding this problem with Internet Explorer, sorry I can't be more specific with versions!

          Show
          Paul Holden added a comment - Frédéric, upload some files with .notebook extension and confirm the mimetype is being stored and sent incorrectly - my patch should fix these on upgrade. After applying the patch, upload another file with .notebook extension and confirm that the correct mimetype is stored in the 'files' table and sent as a header when downloading the file (the Live HTTP Headers Firefox plugin is useful for this). We had numerous reports from members of staff here regarding this problem with Internet Explorer, sorry I can't be more specific with versions!
          Hide
          Frédéric Massart added a comment -

          I understand the issue Paul, and I could see that the headers were wrong, but I would prefer being able to replicate this issue so that it could be test, although if we really cannot reproduce it then we'll have to do without I guess. Also, the patch proposed by Matteo implements other files from SMART which I'd be happy to add to the list but a sample of those files would be ideal.

          Have a nice weekend!
          Fred

          Show
          Frédéric Massart added a comment - I understand the issue Paul, and I could see that the headers were wrong, but I would prefer being able to replicate this issue so that it could be test, although if we really cannot reproduce it then we'll have to do without I guess. Also, the patch proposed by Matteo implements other files from SMART which I'd be happy to add to the list but a sample of those files would be ideal. Have a nice weekend! Fred
          Hide
          Chris Follin added a comment -

          Hi, Fred. I can't send real data files without our client's permission but I have requested that they send us some sample files that can be shared publicly. I'll post them here when or if I receive them.

          Show
          Chris Follin added a comment - Hi, Fred. I can't send real data files without our client's permission but I have requested that they send us some sample files that can be shared publicly. I'll post them here when or if I receive them.
          Hide
          David Broadley added a comment -

          Hi I don't know if you have some test files for smart notebook but here you go!!

          Show
          David Broadley added a comment - Hi I don't know if you have some test files for smart notebook but here you go!!
          Hide
          Chris Follin added a comment -

          Attaching some more test files.

          Show
          Chris Follin added a comment - Attaching some more test files.
          Hide
          Frédéric Massart added a comment -

          Thanks guys! I pulled Matteo's patch and fixed the version number and comment inline. I am uploading a .gallery and a .xbk file that I've found on the net. That would be great if you could test the patch with the files uploaded here on an IE which has the issue.

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Thanks guys! I pulled Matteo's patch and fixed the version number and comment inline. I am uploading a .gallery and a .xbk file that I've found on the net. That would be great if you could test the patch with the files uploaded here on an IE which has the issue. Cheers, Fred
          Hide
          Frédéric Massart added a comment -

          Could you also confirm that .xbk files are SVGs?

          Show
          Frédéric Massart added a comment - Could you also confirm that .xbk files are SVGs?
          Hide
          Paul Holden added a comment -

          Doesn't look like the db/upgrade code from Matteo's patch is correct, testing the upgrade only fixed the mimetypes of .gallery files (ignoring the other extensions) - you need to loop through the extensions and correct each separately, instead of passing them all to $DB->set_field_select

          I've added a commit to https://github.com/paulholden/moodle/compare/master...MDL-35622 that does this.

          Show
          Paul Holden added a comment - Doesn't look like the db/upgrade code from Matteo's patch is correct, testing the upgrade only fixed the mimetypes of .gallery files (ignoring the other extensions) - you need to loop through the extensions and correct each separately, instead of passing them all to $DB->set_field_select I've added a commit to https://github.com/paulholden/moodle/compare/master...MDL-35622 that does this.
          Hide
          Matteo Scaramuccia added a comment -

          Hi Paul,
          mine was just a PoC (no tests at all) to list all the supposed mime types and didn't focus on the update stage and yes I made a big mistake around set_field_select at that time, confusing the list of bound query parameters with the number of times required to apply the update for each extension.

          TNX!
          Matteo

          Show
          Matteo Scaramuccia added a comment - Hi Paul, mine was just a PoC (no tests at all) to list all the supposed mime types and didn't focus on the update stage and yes I made a big mistake around set_field_select at that time, confusing the list of bound query parameters with the number of times required to apply the update for each extension. TNX! Matteo
          Hide
          Frédéric Massart added a comment -

          Wow. Thanks Paul for pointing out the SQL error! I can't believe I missed it... I tried to pull your patch in but it was getting messy, conflicting and confusing. So I took the liberty of creating the patch under my name and credit you both in the commit message. hope you guys are fine with that. Many thanks!

          Show
          Frédéric Massart added a comment - Wow. Thanks Paul for pointing out the SQL error! I can't believe I missed it... I tried to pull your patch in but it was getting messy, conflicting and confusing. So I took the liberty of creating the patch under my name and credit you both in the commit message. hope you guys are fine with that. Many thanks!
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [Y] Databases
          [Y] Testing (are you providing the kit-kats?)
          [-] Security
          [?] Documentation
          [Y] Git
          [Y] Sanity check

          As it seems to be a file api change, does this need documentation?
          Other than that, the code looks fine, and should be good to integrate now.

          Show
          Jason Fowler added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [Y] Databases [Y] Testing (are you providing the kit-kats?) [-] Security [?] Documentation [Y] Git [Y] Sanity check As it seems to be a file api change, does this need documentation? Other than that, the code looks fine, and should be good to integrate now.
          Hide
          Jason Fowler added a comment -

          Spoke to Fred, the api change doesn't need documenting.

          Show
          Jason Fowler added a comment - Spoke to Fred, the api change doesn't need documenting.
          Hide
          Frédéric Massart added a comment -

          Thanks Jason.

          Show
          Frédéric Massart added a comment - Thanks Jason.
          Hide
          Damyon Wiese 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.

          Thanks!

          Show
          Damyon Wiese 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. Thanks!
          Hide
          Dan Poltawski added a comment -

          I was a bit apprehensive about the .gallery file extension, as thats quite a generic name and who knows what else it could be. But i've looked around the internet and I can't find any other file claiming the extension .gallery. The other factor is that we're in education and we know lots of our users use smart boards, so it makes sense to go with this over another file not so common filetype.

          Show
          Dan Poltawski added a comment - I was a bit apprehensive about the .gallery file extension, as thats quite a generic name and who knows what else it could be. But i've looked around the internet and I can't find any other file claiming the extension .gallery. The other factor is that we're in education and we know lots of our users use smart boards, so it makes sense to go with this over another file not so common filetype.
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23, thanks.

          I hope it wasn't my perception that this wasn't rebased, and it actually was cos I had to do more conflict resolution that I liked

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23, thanks. I hope it wasn't my perception that this wasn't rebased, and it actually was cos I had to do more conflict resolution that I liked
          Hide
          Michael de Raadt added a comment -

          Test result: Unsuccessful.

          All worked except the .galleryitem files in Master. These files had a MIME type of document/unknown and an icon to match.

          Show
          Michael de Raadt added a comment - Test result: Unsuccessful. All worked except the .galleryitem files in Master. These files had a MIME type of document/unknown and an icon to match.
          Hide
          Frédéric Massart added a comment -

          Thanks for spotting this Michael. I've added a fix in each branch. Could you please re-test once integrated? You don't need to re-test the other file types.

          Show
          Frédéric Massart added a comment - Thanks for spotting this Michael. I've added a fix in each branch. Could you please re-test once integrated? You don't need to re-test the other file types.
          Hide
          Michael de Raadt added a comment -

          Sure. Let me know when it is ready to go.

          Show
          Michael de Raadt added a comment - Sure. Let me know when it is ready to go.
          Hide
          Dan Poltawski added a comment -

          Thanks Fred, reset back to testing.

          Show
          Dan Poltawski added a comment - Thanks Fred, reset back to testing.
          Hide
          Michael de Raadt added a comment -

          Test result: Success!

          *.galleryitem files now tow-the-line.

          Show
          Michael de Raadt added a comment - Test result: Success! *.galleryitem files now tow-the-line.
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon

            People

            • Votes:
              5 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: