Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42476

Completion report should use feature detection rather than browser sniffing

          Details

          • Testing Instructions:
            Hide

            In as many browsers as possible (at least IE>9, IE8, firefox, chrome, mobilesafari)

            1. Enable completion tracking in advanced features
            2. In a course, enable completion tracking in the course settings
            3. Go to "completion tracking" and enable self completion and completion by date
            4. Go to course -> reports > course completion
            5. Enrol a user in the course
            6. VERIFY: no JS errors occur
            7. VERIFY: the text is rotated if the browser supports SVG (IE9+ and all the rest)
            8. VERIFY: the text is not rotated if the browser doesn't support SVG (IE8).
            Show
            In as many browsers as possible (at least IE>9, IE8, firefox, chrome, mobilesafari) Enable completion tracking in advanced features In a course, enable completion tracking in the course settings Go to "completion tracking" and enable self completion and completion by date Go to course -> reports > course completion Enrol a user in the course VERIFY: no JS errors occur VERIFY: the text is rotated if the browser supports SVG (IE9+ and all the rest) VERIFY: the text is not rotated if the browser doesn't support SVG (IE8).
          • Affected Branches:
            MOODLE_24_STABLE, MOODLE_25_STABLE
          • Fixed Branches:
            MOODLE_24_STABLE, MOODLE_25_STABLE
          • Pull from Repository:
          • Pull Master Branch:
          • Pull Master Diff URL:

            Description

            Same issue as MDL-39964 but with completion report.

            Browser sniffing is problematic and should be avoiding. Since this feature uses JS to do its work, we should be able to use feature detection.

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Thanks for working on this, Aaron. Hopefully it will be picked up for review soon.

                  Show
                  salvetore Michael de Raadt added a comment - Thanks for working on this, Aaron. Hopefully it will be picked up for review soon.
                  Hide
                  Permalink
                  phalacee Jason Fowler added a comment -

                  [Y] Syntax
                  [Y] Whitespace
                  [-] Output
                  [-] Language
                  [-] Databases
                  [Y] Testing (instructions and automated tests)
                  [-] Security
                  [-] Documentation
                  [Y] Git
                  [-] Third party code
                  [N] Sanity check

                  Thanks Aaron, great work, just one question, what is going on with the changes at line 29 in the diff? Why is that being removed? It seems unrelated to the rest of the patch.

                  Show
                  phalacee Jason Fowler added a comment - [Y] Syntax [Y] Whitespace [-] Output [-] Language [-] Databases [Y] Testing (instructions and automated tests) [-] Security [-] Documentation [Y] Git [-] Third party code [N] Sanity check Thanks Aaron, great work, just one question, what is going on with the changes at line 29 in the diff? Why is that being removed? It seems unrelated to the rest of the patch.
                  Hide
                  Permalink
                  sry_not4sale Aaron Barnes added a comment -

                  Hi Jason,

                  The removed code is some Totara-specific code that shouldn't have made it into Moodle at all.

                  Cheers,
                  Aaron

                  Show
                  sry_not4sale Aaron Barnes added a comment - Hi Jason, The removed code is some Totara-specific code that shouldn't have made it into Moodle at all. Cheers, Aaron
                  Hide
                  Permalink
                  phalacee Jason Fowler added a comment -

                  Well that explains why I couldn't make head nor tail of how it was connected to the rest of the code. Thank Aaron, pushing for integration now then.

                  Show
                  phalacee Jason Fowler added a comment - Well that explains why I couldn't make head nor tail of how it was connected to the rest of the code. Thank Aaron, pushing for integration now then.
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  Hmm, I guess I didn't detect this in the original issue because weren't detecting it originally?

                  Show
                  poltawski Dan Poltawski added a comment - Hmm, I guess I didn't detect this in the original issue because weren't detecting it originally?
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  Thanks Aaron, integrated to master, 25 and 24

                  Show
                  poltawski Dan Poltawski added a comment - Thanks Aaron, integrated to master, 25 and 24
                  Hide
                  Permalink
                  rwijaya Rossiani Wijaya added a comment -

                  This is working as expected.

                  Tested for 2.4, 2.5 and master with IE8,9,10,11, FF, Chrome, and safarimobile.

                  All text displayed vertically in all browsers, except IE8.

                  Test passed.

                  Show
                  rwijaya Rossiani Wijaya added a comment - This is working as expected. Tested for 2.4, 2.5 and master with IE8,9,10,11, FF, Chrome, and safarimobile. All text displayed vertically in all browsers, except IE8. Test passed.
                  Hide
                  Permalink
                  damyon Damyon Wiese added a comment -

                  Here lies 52 bugs.
                  All fixed or swept under a rug.
                  If they come back one day,
                  To our dismay,
                  We all will feel quite un-smug.

                  Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

                  Show
                  damyon Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

                    People

                    • Votes:
                      0 Vote for this issue
                      Watchers:
                      4 Start watching this issue

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        11/Nov/13