Moodle
  1. Moodle
  2. MDL-35469

Moodle is incorrectly detecting the user agent strings of Firefox alpha

    Details

    • Testing Instructions:
      Hide

      1/ run phpunit tests (in 23 and master)
      2/ install stable version of firefox, verify editor works
      3/ install latest firefox nightly and verify editor and ajax stuff works fine

      Show
      1/ run phpunit tests (in 23 and master) 2/ install stable version of firefox, verify editor works 3/ install latest firefox nightly and verify editor and ajax stuff works fine
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w42_MDL-35469_m24_geckover
    • Rank:
      44161

      Description

      I have seen a problem with the TinyMCE editor not displaying inside Firefox 17, it just stays as a textarea with a select box below it saying "HTML Format". It works perfectly in Firefox 16 and Chrome.

      Its occurring in all themes on all pages with the editor.

      I have:

      • Cleared the cache.
      • Used new profile.
      • Looked at a number of moodle settings.

        Issue Links

          Activity

          Hide
          Hugh Nougher added a comment -
          Show
          Hugh Nougher added a comment - For reference: https://bugzilla.mozilla.org/show_bug.cgi?id=797703
          Hide
          Gervase Markham added a comment -

          This looks like the problem to me:

          https://github.com/moodle/moodle/blob/5d31799af81d7f9fdb03d062f4ac507c08c4d77d/course/format/weeks/ajax.php

          https://github.com/moodle/moodle/blob/5d31799af81d7f9fdb03d062f4ac507c08c4d77d/course/format/topics/ajax.php

          (And there seem to be two other files in the repo with similar names and code. The Gecko date was frozen several years ago, and is no longer a reliable guide to anything. We recently changed it to a version number, but obviously a numeric comparison sees this as "old". If you really need this check (surely everyone is now using a version of Firefox new enough for Ajax?), then you should check the "rv:" version number (common to all Gecko-based browsers), not the now-no-longer-present Gecko date.

          Gerv

          Show
          Gervase Markham added a comment - This looks like the problem to me: https://github.com/moodle/moodle/blob/5d31799af81d7f9fdb03d062f4ac507c08c4d77d/course/format/weeks/ajax.php https://github.com/moodle/moodle/blob/5d31799af81d7f9fdb03d062f4ac507c08c4d77d/course/format/topics/ajax.php (And there seem to be two other files in the repo with similar names and code. The Gecko date was frozen several years ago, and is no longer a reliable guide to anything. We recently changed it to a version number, but obviously a numeric comparison sees this as "old". If you really need this check (surely everyone is now using a version of Firefox new enough for Ajax?), then you should check the "rv:" version number (common to all Gecko-based browsers), not the now-no-longer-present Gecko date. Gerv
          Hide
          Gervase Markham added a comment -

          Can someone responsible for this issue contact us ASAP in the Mozilla bug, because we need to work out how to best mitigate the problem for existing Moodle installations.

          Thanks!

          Gerv

          Show
          Gervase Markham added a comment - Can someone responsible for this issue contact us ASAP in the Mozilla bug, because we need to work out how to best mitigate the problem for existing Moodle installations. Thanks! Gerv
          Hide
          Petr Škoda added a comment -

          Thanks a lot for the report and pointers to the solution. I suppose we can patch all versions, hopefully all admins upgrade Moodle regularly at least for security reasons.

          Show
          Petr Škoda added a comment - Thanks a lot for the report and pointers to the solution. I suppose we can patch all versions, hopefully all admins upgrade Moodle regularly at least for security reasons.
          Hide
          Gervase Markham added a comment -

          Thank you. I hope that's true too... We need to figure out whether we need to put mitigations into Firefox.

          Are you planning to put this fix on to all stable branches? What kind of uptake rates do you see for security releases? Are you, as the Moodle community, happy for us not to put mitigations into Firefox and for this to serve as an upgrade driver? If we were to put mitigations in, is there a good way of detecting we are talking to Moodle? Someone has suggested looking at cookies...

          Gerv

          Show
          Gervase Markham added a comment - Thank you. I hope that's true too... We need to figure out whether we need to put mitigations into Firefox. Are you planning to put this fix on to all stable branches? What kind of uptake rates do you see for security releases? Are you, as the Moodle community, happy for us not to put mitigations into Firefox and for this to serve as an upgrade driver? If we were to put mitigations in, is there a good way of detecting we are talking to Moodle? Someone has suggested looking at cookies... Gerv
          Hide
          Petr Škoda added a comment -

          Hi, I have implemented a patch suitable for all our stable releases and even unsupported versions. Our upgrade rates are always lower than what we would want. Our next releases are planned to be done in December.

          I am going to post in our user forums on moodle.org once I get my patches tested, let's see what feedback we get there first.

          Show
          Petr Škoda added a comment - Hi, I have implemented a patch suitable for all our stable releases and even unsupported versions. Our upgrade rates are always lower than what we would want. Our next releases are planned to be done in December. I am going to post in our user forums on moodle.org once I get my patches tested, let's see what feedback we get there first.
          Hide
          Martin Dougiamas added a comment -

          Hi all, thanks all for working on this.

          If we can get that "mitigation" in Firefox then it would be good. We would also push our patch out to the Moodle community and educate people to upgrade ASAP but Firefox support (even for a while) would help a lot of schools without good technical support.

          For code niceness, you could remove the patch from Firefox in a year or so.

          Show
          Martin Dougiamas added a comment - Hi all, thanks all for working on this. If we can get that "mitigation" in Firefox then it would be good. We would also push our patch out to the Moodle community and educate people to upgrade ASAP but Firefox support (even for a while) would help a lot of schools without good technical support. For code niceness, you could remove the patch from Firefox in a year or so.
          Hide
          Petr Škoda added a comment -

          Hmm, it should be safe to identify all Moodle sites by looking for /^MoodleSession/ cookies.

          Show
          Petr Škoda added a comment - Hmm, it should be safe to identify all Moodle sites by looking for /^MoodleSession/ cookies.
          Hide
          Dan Poltawski added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.
          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.
          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P
          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Dan Poltawski added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee 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
          Aparup Banerjee 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 - - edited

          My +1 to backport this to all supported versions (security-only and fully supported).

          Aka: 19, 21, 22, 23 and master (keeping 20, totally unsupported out).

          PS: And yes, I know it's an exception for 19 and 21, but...

          Show
          Eloy Lafuente (stronk7) added a comment - - edited My +1 to backport this to all supported versions (security-only and fully supported). Aka: 19, 21, 22, 23 and master (keeping 20, totally unsupported out). PS: And yes, I know it's an exception for 19 and 21, but...
          Hide
          Dan Marsden added a comment -

          I'm not sure about backporting this to 1.9 myself...

          Eloy tells me I can only vote using integers.... so my vote is -0

          Reasons:

          • We only support 1.9 for serious security issues.
          • support for FF17 is a good reason to encourage people to upgrade from 1.9
          • textarea is usable even when tinymce isn't displayed.

          but - happy for integrators to override my -0 and integrate it anyway - esp as the patch is already done.

          Show
          Dan Marsden added a comment - I'm not sure about backporting this to 1.9 myself... Eloy tells me I can only vote using integers.... so my vote is -0 Reasons: We only support 1.9 for serious security issues. support for FF17 is a good reason to encourage people to upgrade from 1.9 textarea is usable even when tinymce isn't displayed. but - happy for integrators to override my -0 and integrate it anyway - esp as the patch is already done.
          Hide
          Dan Poltawski added a comment -

          My -1 for 19 too

          Show
          Dan Poltawski added a comment - My -1 for 19 too
          Hide
          Dan Poltawski added a comment - - edited

          Why are we even UA sniffing for tinymce anyway? Does tinymce do feature detection itself? The tinymce chart doesn't even mention firefox versions:
          http://www.tinymce.com/wiki.php/Browser_compatiblity

          Really I think we really should be eliminating the sniffing as a long-term solution to these kind of problems. I have created MDL-36316 for that.

          Show
          Dan Poltawski added a comment - - edited Why are we even UA sniffing for tinymce anyway? Does tinymce do feature detection itself? The tinymce chart doesn't even mention firefox versions: http://www.tinymce.com/wiki.php/Browser_compatiblity Really I think we really should be eliminating the sniffing as a long-term solution to these kind of problems. I have created MDL-36316 for that.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I agree with Dan about to take rid of unnecesary UA sniffing where possible, so my +1 for MDL-36316.

          Unlucky we still have to decide about this now.

          Show
          Eloy Lafuente (stronk7) added a comment - I agree with Dan about to take rid of unnecesary UA sniffing where possible, so my +1 for MDL-36316 . Unlucky we still have to decide about this now.
          Hide
          Matthew N added a comment -

          "textarea is usable even when tinymce isn't displayed."

          I don't think this is the case because most teachers don't know HTML and won't be able to understand the HTML previously generated by TinyMCE for existing content.

          Also, most teachers will want to format text and they won't be able to do that effectively without an editor. Since over 50% of installs still seem to be running 1.9 I think many users will continue to suffer if this is not fixed.

          +1 for 1.9

          Show
          Matthew N added a comment - "textarea is usable even when tinymce isn't displayed." I don't think this is the case because most teachers don't know HTML and won't be able to understand the HTML previously generated by TinyMCE for existing content. Also, most teachers will want to format text and they won't be able to do that effectively without an editor. Since over 50% of installs still seem to be running 1.9 I think many users will continue to suffer if this is not fixed. +1 for 1.9
          Hide
          Dan Marsden added a comment -

          Hi Matthew,
          support for bugfixing in Moodle 1.9 ended June 2011 - we do support security fixes until Dec 2013 - this bug doesn't qualify as a security issue so I'm changing my vote from -0 to -1

          If people really want to use 1.9 and include bug fixes they should use git to manage their installation and cherry pick/merge in their own changes.

          you are also welcome to use our mdl19-local branch at catalyst which includes further fixes that aren't included in the core release (but it also includes some specific Catalyst patches for our systems)
          http://git.catalyst.net.nz/gw?p=moodle-r2.git;a=shortlog;h=refs/heads/mdl19-local

          Show
          Dan Marsden added a comment - Hi Matthew, support for bugfixing in Moodle 1.9 ended June 2011 - we do support security fixes until Dec 2013 - this bug doesn't qualify as a security issue so I'm changing my vote from -0 to -1 If people really want to use 1.9 and include bug fixes they should use git to manage their installation and cherry pick/merge in their own changes. you are also welcome to use our mdl19-local branch at catalyst which includes further fixes that aren't included in the core release (but it also includes some specific Catalyst patches for our systems) http://git.catalyst.net.nz/gw?p=moodle-r2.git;a=shortlog;h=refs/heads/mdl19-local
          Hide
          Matthew N added a comment -

          I'm aware of the support timeline and I successfully upgraded to 2.3, albeit with hardware upgrades to handle the increased resource usage, but I know of other sites which won't be upgraded past 1.9 before next summer. The fact is that many installs are still running 1.9 and most schools won't do major upgrades in the middle of the school year.

          This is a low risk change for a high value reward to fix a bad practice (UA detection).

          I'm looking forward to MDL-36316 for the proper solution as I agree that the proposed pull requests just workaround the problem.

          Show
          Matthew N added a comment - I'm aware of the support timeline and I successfully upgraded to 2.3, albeit with hardware upgrades to handle the increased resource usage, but I know of other sites which won't be upgraded past 1.9 before next summer. The fact is that many installs are still running 1.9 and most schools won't do major upgrades in the middle of the school year. This is a low risk change for a high value reward to fix a bad practice (UA detection). I'm looking forward to MDL-36316 for the proper solution as I agree that the proposed pull requests just workaround the problem.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          Martin Dougiamas added a comment -

          My +10 for 1.9 as well. The HTML editor is a pretty key thing and it's a small patch.

          Show
          Martin Dougiamas added a comment - My +10 for 1.9 as well. The HTML editor is a pretty key thing and it's a small patch.
          Hide
          Dan Poltawski added a comment -

          OK, i'm going to integrate this to 1.9 too rather than delay this any longer

          Show
          Dan Poltawski added a comment - OK, i'm going to integrate this to 1.9 too rather than delay this any longer
          Hide
          Dan Poltawski added a comment - - edited

          The firefox nighly already has an override for Moodle, so this is actually pretty tricky to test

          Actually, just found the about:config pref for it: general.useragent.complexOverride.moodle

          Show
          Dan Poltawski added a comment - - edited The firefox nighly already has an override for Moodle, so this is actually pretty tricky to test Actually, just found the about:config pref for it: general.useragent.complexOverride.moodle
          Hide
          Dan Poltawski added a comment - - edited

          I've integrated and tested this now.

          To test

          • I used the latest nightly build
          • I used about:config to disable general.useragent.complexOverride.moodle
          • I verified that the editor was not loading
          • I applied the patch
          • I verified that the editor then loaded
          • I verified that the stable versions of firefox were also still working.

          BIG thanks to the Firefox team for working around our bugs for the benefit of both our users

          Show
          Dan Poltawski added a comment - - edited I've integrated and tested this now. To test I used the latest nightly build I used about:config to disable general.useragent.complexOverride.moodle I verified that the editor was not loading I applied the patch I verified that the editor then loaded I verified that the stable versions of firefox were also still working. BIG thanks to the Firefox team for working around our bugs for the benefit of both our users
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Kudos to FF's team (should I consider using it as my primary browser?). LOL.

          Thanks everybody for the quick handling of this!

          Show
          Eloy Lafuente (stronk7) added a comment - Kudos to FF's team (should I consider using it as my primary browser?). LOL. Thanks everybody for the quick handling of this!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: