[MDL-28972] html_quickform_toggle_checkboxes use getElementsByClassName which is not supported by ie - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: 2.0.4, 2.1.1
Fix Version/s: 2.1.6, 2.2.3
Component/s: Forms Library
Labels:
- triaged

Testing Instructions:
Hide

Test needs to be done on IE, FF, chrome, Safari and any other browser you may have.
Test 1:
Use the attached test script.
When you click "select all/none", the checkbox state should toggle.
Disable JS, reload page and click "Select all/none"
Test 2:

Go to a course

Click on Settings > Grades

Click on Settings > Grade Administration > Export > Excel

In the Grade items to be included fieldset at the bottom, click Select all/none

checkbox should toggle

Try above with JS disabled.
Show
Test needs to be done on IE, FF, chrome, Safari and any other browser you may have. Test 1: Use the attached test script. When you click "select all/none", the checkbox state should toggle. Disable JS, reload page and click "Select all/none" Test 2: Go to a course Click on Settings > Grades Click on Settings > Grade Administration > Export > Excel In the Grade items to be included fieldset at the bottom, click Select all/none checkbox should toggle Try above with JS disabled.
Affected Branches:
MOODLE_20_STABLE, MOODLE_21_STABLE
Fixed Branches:
MOODLE_21_STABLE, MOODLE_22_STABLE
Pull from Repository:
git://github.com/rajeshtaneja/moodle.git
Pull Master Branch:
wip-mdl-28972
Pull Master Diff URL:
https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-28972

Description

While testing ~~MDL-28737~~, it was observed that html_quickform_toggle_checkboxes is using getElementsByClassName. This is not supported in ie.

Message: Object doesn't support this property or method

Line: 14

Char: 5

Code: 0

URI: http://rajesh.moodle.local/moodle/test.php

Gliffy Diagrams

Attachments

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Thumbnails
- List
- Download All

Attachments

test.php
1 kB
08/Mar/12 5:48 PM

Issue Links

has been marked as being related by

MDL-28737 advcheckboxes toggle their value every form submit in the presence of a checkbox_controller

Closed

is duplicated by

MDL-31467 Select All Checkboxes does not work in IE8

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Tim Hunt added a comment - 17/Aug/11 5:51 PM

Hmm. If you search for document.getElementsByTagName, you sill see that we are using getElementsByTagName in lots of places, not just the one occurrence I added. Big nasty clean-up job for someone.

Show

Tim Hunt added a comment - 17/Aug/11 5:51 PM Hmm. If you search for document.getElementsByTagName, you sill see that we are using getElementsByTagName in lots of places, not just the one occurrence I added. Big nasty clean-up job for someone.

Hide

Permalink

Simon Clegg added a comment - 09/Feb/12 9:21 PM

I've recently reported a related bug ~~MDL-31467~~
Is there a progress on a resolution for the root of this issue?

Show

Simon Clegg added a comment - 09/Feb/12 9:21 PM I've recently reported a related bug MDL-31467 Is there a progress on a resolution for the root of this issue?

Hide

Permalink

Rajesh Taneja added a comment - 10/Feb/12 9:29 AM

Added Michael as watcher, so it can be pushed in next sprint.
Also, I think priority should be increased to major.

Show

Rajesh Taneja added a comment - 10/Feb/12 9:29 AM Added Michael as watcher, so it can be pushed in next sprint. Also, I think priority should be increased to major.

Hide

Permalink

Rajesh Taneja added a comment - 08/Mar/12 5:50 PM

Submit link with JS disabled is not working and orignalvalue parameter is not used in add_checkbox_controller, so should be handled in separate bug.

Show

Rajesh Taneja added a comment - 08/Mar/12 5:50 PM Submit link with JS disabled is not working and orignalvalue parameter is not used in add_checkbox_controller, so should be handled in separate bug.

Hide

Permalink

Rajesh Taneja added a comment - 09/Mar/12 3:51 PM

Following fixed:

checkbox controller JS is using YUI class.
Since 20 non-JS checkbox controller was broken (select all/none in non-JS mode not working), is fixed now.
Moved code to make it readable in checkboxcontroller function

Show

Rajesh Taneja added a comment - 09/Mar/12 3:51 PM Following fixed: checkbox controller JS is using YUI class. Since 20 non-JS checkbox controller was broken (select all/none in non-JS mode not working), is fixed now. Moved code to make it readable in checkboxcontroller function

Hide

Permalink

Jason Fowler added a comment - 12/Mar/12 11:27 AM

code looks fine, just update the test instructions to cover the scope of the changes made

Show

Jason Fowler added a comment - 12/Mar/12 11:27 AM code looks fine, just update the test instructions to cover the scope of the changes made

Hide

Permalink

Rajesh Taneja added a comment - 12/Mar/12 11:54 AM

Thanks Jason,

Updated test instructions.

Show

Rajesh Taneja added a comment - 12/Mar/12 11:54 AM Thanks Jason, Updated test instructions.

Hide

Permalink

Sam Hemelryk added a comment - 14/Mar/12 9:06 AM

This will likely be my last review for the day

Show

Sam Hemelryk added a comment - 14/Mar/12 9:06 AM This will likely be my last review for the day

Hide

Permalink

Sam Hemelryk added a comment - 14/Mar/12 9:41 AM

Thanks Raj, this has been integrated now

I did make one additional commit on master to fix up the variable names within that function.

Cheers
Sam

Show

Sam Hemelryk added a comment - 14/Mar/12 9:41 AM Thanks Raj, this has been integrated now I did make one additional commit on master to fix up the variable names within that function. Cheers Sam

Hide

Permalink

Rajesh Taneja added a comment - 14/Mar/12 10:29 AM

Thanks Sam.

Show

Rajesh Taneja added a comment - 14/Mar/12 10:29 AM Thanks Sam.

Hide

Permalink

Adrian Greeve added a comment - 14/Mar/12 11:46 AM

Tested on 2.1, 2.2 and master on IE, Firefox, Chrome and Safari. Works fine. No probblems.
Thanks.

Show

Adrian Greeve added a comment - 14/Mar/12 11:46 AM Tested on 2.1, 2.2 and master on IE, Firefox, Chrome and Safari. Works fine. No probblems. Thanks.

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 15/Mar/12 8:23 PM

FCT (fixed, closing, thanks). Ciao

"I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
~ Benjamin Disraeli

Show

Eloy Lafuente (stronk7) added a comment - 15/Mar/12 8:23 PM FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

People

Assignee:

Rajesh Taneja

Reporter:

Rajesh Taneja

Peer reviewer:

Jason Fowler

Integrator:

Sam Hemelryk

Tester:

Adrian Greeve

Participants:

Adrian Greeve, Eloy Lafuente (stronk7), Jason Fowler, Rajesh Taneja, Sam Hemelryk, Simon Clegg, Tim Hunt

Votes:

2 Vote for this issue

Watchers:

0 Start watching this issue

Dates

Created:

17/Aug/11 4:34 PM

Updated:

30/Sep/13 10:15 AM

Resolved:

15/Mar/12 8:23 PM

Fix Release Date:

14/May/12