Moodle
  1. Moodle
  2. MDL-23224

Adding blocks doesn't work in Chrome

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Duplicate
    • Affects Version/s: 2.0.1
    • Fix Version/s: None
    • Component/s: Blocks
    • Labels:
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE

      Description

      In Google Chrome browser adding blocks seems to be broken. You can select the new block from the dropdown but then nothing happens.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Simon Coggins added a comment -

            I found that selecting the option fails to auto-submit the form, and leaves the pulldown focused. As soon as you move the focus off the selected element the onBlur event fires and it submits as expected. So the problems seems to be related to the select.on('click', ...) function called in lib/javascript-static.js:

            // Attach the change event to the keypress, blur, and click actions.
            // We don't use the change event because IE fires it on every arrow up/down
            // event.... usability
            Y.on('key', processchange, select, 'press:13', form, select.get('selectedIndex'));
            select.on('blur', processchange, form, select.get('selectedIndex'));
            select.on('click', processchange, form, select.get('selectedIndex'));
            

            In chrome the click event doesn't seem to be firing.

            Show
            Simon Coggins added a comment - I found that selecting the option fails to auto-submit the form, and leaves the pulldown focused. As soon as you move the focus off the selected element the onBlur event fires and it submits as expected. So the problems seems to be related to the select.on('click', ...) function called in lib/javascript-static.js: // Attach the change event to the keypress, blur, and click actions. // We don't use the change event because IE fires it on every arrow up/down // event.... usability Y.on( 'key' , processchange, select, 'press:13' , form, select.get( 'selectedIndex' )); select.on( 'blur' , processchange, form, select.get( 'selectedIndex' )); select.on( 'click' , processchange, form, select.get( 'selectedIndex' )); In chrome the click event doesn't seem to be firing.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Thanks for the report, moving to stable backlog to be fixed asap. Raising to critical.

            Show
            Eloy Lafuente (stronk7) added a comment - Thanks for the report, moving to stable backlog to be fixed asap. Raising to critical.
            Hide
            Jérôme Mouneyrac added a comment -

            There is an issue for that for Chromium: http://code.google.com/p/chromium/issues/detail?id=5284&q=onChange%20dropdown&colspec=ID%20Stars%20Pri%20Area%20Feature%20Type%20Status%20Summary%20Modified%20Owner%20Mstone%20OS

            Note that it works with my Safari, so it could have been fixed. The latest Chrome (9) and lastest chromium (from 4/1/11) versions don't work.

            We found a fix with Dongsheng that should keep it working on all browser:

            [code]
            //little hack for chrome that need on change
            if (Y.UA.webkit)

            { select.on('change', processchange, form, select.get('selectedIndex')); }

            else

            { select.on('click', processchange, form, select.get('selectedIndex')); }

            [code]

            Show
            Jérôme Mouneyrac added a comment - There is an issue for that for Chromium: http://code.google.com/p/chromium/issues/detail?id=5284&q=onChange%20dropdown&colspec=ID%20Stars%20Pri%20Area%20Feature%20Type%20Status%20Summary%20Modified%20Owner%20Mstone%20OS Note that it works with my Safari, so it could have been fixed. The latest Chrome (9) and lastest chromium (from 4/1/11) versions don't work. We found a fix with Dongsheng that should keep it working on all browser: [code] //little hack for chrome that need on change if (Y.UA.webkit) { select.on('change', processchange, form, select.get('selectedIndex')); } else { select.on('click', processchange, form, select.get('selectedIndex')); } [code]
            Hide
            Sam Hemelryk added a comment -

            Hehe sorry Jerome, just saw you grabbed this one.
            Try the following patch

            diff --git a/lib/javascript-static.js b/lib/javascript-static.js
            index d79230d..76596c9 100644
            --- a/lib/javascript-static.js
            +++ b/lib/javascript-static.js
            @@ -347,7 +347,7 @@ M.util.init_select_autosubmit = function(Y, formid, selectid, nothing) {
                             // We need to manually iterate at this point because if the case
                             // above is true YUI's ancestor method will also kill IE!
                             var form = select;
            -                while (form && form.get('nodeName').toUpperCase() !== 'FORM') {
            +                while (form && !form.test('form')) {
                                 form = form.ancestor();
                             }
                             return form;
            @@ -360,12 +360,17 @@ M.util.init_select_autosubmit = function(Y, formid, selectid, nothing) {
                                     this.submit();
                                 }
                             };
            +
                             // Attach the change event to the keypress, blur, and click actions.
            -                // We don't use the change event because IE fires it on every arrow up/down
            -                // event.... usability
                             Y.on('key', processchange, select, 'press:13', form, select.get('selectedIndex'));
            -                select.on('blur', processchange, form, select.get('selectedIndex'));
            -                select.on('click', processchange, form, select.get('selectedIndex'));
            +                if (Y.UA.ie > 0) {
            +                    // We don't use the change event because IE fires it on every arrow up/down
            +                    // event.... usability
            +                    select.on('blur', processchange, form, select.get('selectedIndex'));
            +                    select.on('click', processchange, form, select.get('selectedIndex'));
            +                } else {
            +                    select.on('change', processchange, form, select.get('selectedIndex'));      
            +                }
                         }
                     }
                 });
            

            Show
            Sam Hemelryk added a comment - Hehe sorry Jerome, just saw you grabbed this one. Try the following patch diff --git a/lib/javascript-static.js b/lib/javascript-static.js index d79230d..76596c9 100644 --- a/lib/javascript-static.js +++ b/lib/javascript-static.js @@ -347,7 +347,7 @@ M.util.init_select_autosubmit = function(Y, formid, selectid, nothing) { // We need to manually iterate at this point because if the case // above is true YUI's ancestor method will also kill IE! var form = select; - while (form && form.get('nodeName').toUpperCase() !== 'FORM') { + while (form && !form.test('form')) { form = form.ancestor(); } return form; @@ -360,12 +360,17 @@ M.util.init_select_autosubmit = function(Y, formid, selectid, nothing) { this.submit(); } }; + // Attach the change event to the keypress, blur, and click actions. - // We don't use the change event because IE fires it on every arrow up/down - // event.... usability Y.on('key', processchange, select, 'press:13', form, select.get('selectedIndex')); - select.on('blur', processchange, form, select.get('selectedIndex')); - select.on('click', processchange, form, select.get('selectedIndex')); + if (Y.UA.ie > 0) { + // We don't use the change event because IE fires it on every arrow up/down + // event.... usability + select.on('blur', processchange, form, select.get('selectedIndex')); + select.on('click', processchange, form, select.get('selectedIndex')); + } else { + select.on('change', processchange, form, select.get('selectedIndex')); + } } } });
            Hide
            Sam Hemelryk added a comment -

            Haha going to fast for my own good I see you've already worked something similar out.

            Show
            Sam Hemelryk added a comment - Haha going to fast for my own good I see you've already worked something similar out.
            Hide
            Jérôme Mouneyrac added a comment -
            Show
            Jérôme Mouneyrac added a comment - Ah Sam as you already see it you can peer review https://github.com/mouneyrac/moodle/commit/1ede3cb37ffc744da99775570fb19beb69d2fc36
            Hide
            Jérôme Mouneyrac added a comment -

            Just a note, it works in 1.9.

            Show
            Jérôme Mouneyrac added a comment - Just a note, it works in 1.9.
            Hide
            Jérôme Mouneyrac added a comment -

            dongsheng can you peer review too?

            Show
            Jérôme Mouneyrac added a comment - dongsheng can you peer review too?
            Hide
            Dongsheng Cai added a comment -

            Peer review

            Looks good +1

            Show
            Dongsheng Cai added a comment - Peer review Looks good +1
            Hide
            Jérôme Mouneyrac added a comment -

            fixed by MDL-25851

            Show
            Jérôme Mouneyrac added a comment - fixed by MDL-25851

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: