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

Adding blocks doesn't work in Chrome

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              simoncoggins 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
              simoncoggins 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

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

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for the report, moving to stable backlog to be fixed asap. Raising to critical.
              Hide
              jerome 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
              jerome 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
              samhemelryk 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
              samhemelryk 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
              samhemelryk Sam Hemelryk added a comment -

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

              Show
              samhemelryk Sam Hemelryk added a comment - Haha going to fast for my own good I see you've already worked something similar out.
              Hide
              jerome Jérôme Mouneyrac added a comment -
              Show
              jerome 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
              jerome Jérôme Mouneyrac added a comment -

              Just a note, it works in 1.9.

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

              dongsheng can you peer review too?

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

              Peer review

              Looks good +1

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

              fixed by MDL-25851

              Show
              jerome 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: