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
    • Rank:
      2718

      Description

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

        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: