[MDL-32388] Chrome: Scorm open in window, window opens minimized - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 1.9.17, 2.1, 2.2
Fix Version/s: 1.9.18, 2.1.6, 2.2.3
Component/s: SCORM
Labels:

Testing Instructions:

Hide

Add a SCORM to your site - doesn't matter which but here's one:
http://moodle.org/mod/data/view.php?d=50&rid=1655
In the settings choose to open in New Window (it's an advanced option and will be hidden by default)
For first test set 100/100 in the height/width (100 and lower values are interpreted as %)
2nd test use values higher than 100 eg 1024/800 (interpreted as pixels)

Load the SCORM - (with pop-up blockers disabled) - when using Chrome the window is opened in a 0x0 box so you can't see it unless you are looking at the taskbar - the patch attached fixes this.

It would be good to test the "fixed" code on multiple browsers to make sure no regressions are caused. - I have tested the new code in IE8/Chrome/FF

Show
Add a SCORM to your site - doesn't matter which but here's one: http://moodle.org/mod/data/view.php?d=50&rid=1655 In the settings choose to open in New Window (it's an advanced option and will be hidden by default) For first test set 100/100 in the height/width (100 and lower values are interpreted as %) 2nd test use values higher than 100 eg 1024/800 (interpreted as pixels) Load the SCORM - (with pop-up blockers disabled) - when using Chrome the window is opened in a 0x0 box so you can't see it unless you are looking at the taskbar - the patch attached fixes this. It would be good to test the "fixed" code on multiple browsers to make sure no regressions are caused. - I have tested the new code in IE8/Chrome/FF
Affected Branches:
MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
Fixed Branches:
MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
Pull from Repository:
git://github.com/danmarsden/moodle.git
Pull Master Branch:
master_~~MDL-32388~~
Pull Master Diff URL:
https://github.com/danmarsden/moodle/compare/master...master_MDL-32388

Description

Using 2.2.2+ (Build: 20120405)

In chrome (latest 18) when you set the SCORM to open in a new window, the new window opens minimized. Even clicking on the window in task bar doesn't maximize the window. You have to right click the chrome window in task bar and click "maximize" in the right click menu.

Gliffy Diagrams

Attachments

Issue Links

Discovered while testing

MDL-32506 "Allow window to be resized" is an obsolete setting

Closed

MDL-32507 Strict standard errors in scrom Module

Closed

has been marked as being related by

MDL-34590 Quiz secure popup not working in Chrome

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Dan Marsden added a comment - 11/Apr/12 6:50 AM

just saw a post from someone mentioning this might be due to Chrome ignoring the resizeTo() Javascript function - I'll try to get a patch before next weeks integration.

Show

Dan Marsden added a comment - 11/Apr/12 6:50 AM just saw a post from someone mentioning this might be due to Chrome ignoring the resizeTo() Javascript function - I'll try to get a patch before next weeks integration.

Hide

Permalink

Dan Marsden added a comment - 11/Apr/12 4:55 PM

this fixes the issue although I'm not sure it's the "right" fix....
I can't see why resizeTo() was used in the first place? - do some browsers not set the correct size when using them as options in window.open? - I'll try to do some research but it appears to work in FF/Chrome/IE 8

I'm wondering if the JS should be simplified a bit more too - we could calculate the options string in php and just pass the width/height as correct values to JS rather than doing the calculation in the js... putting up for peer review to get someone elses thoughts on this...

Show

Dan Marsden added a comment - 11/Apr/12 4:55 PM this fixes the issue although I'm not sure it's the "right" fix.... I can't see why resizeTo() was used in the first place? - do some browsers not set the correct size when using them as options in window.open? - I'll try to do some research but it appears to work in FF/Chrome/IE 8 I'm wondering if the JS should be simplified a bit more too - we could calculate the options string in php and just pass the width/height as correct values to JS rather than doing the calculation in the js... putting up for peer review to get someone elses thoughts on this...

Hide

Permalink

Dan Marsden added a comment - 11/Apr/12 4:57 PM

heh brain fade moment - we can't use php to calc the width/height - it uses screen.availHeight to do that... doh.

Show

Dan Marsden added a comment - 11/Apr/12 4:57 PM heh brain fade moment - we can't use php to calc the width/height - it uses screen.availHeight to do that... doh.

Hide

Permalink

Dan Marsden added a comment - 11/Apr/12 5:00 PM

..and there doesn't seem to be a reason resizeto was used either - this seems supported in all normal browsers.

Show

Dan Marsden added a comment - 11/Apr/12 5:00 PM ..and there doesn't seem to be a reason resizeto was used either - this seems supported in all normal browsers.

Hide

Permalink

Dan Marsden added a comment - 11/Apr/12 5:13 PM

attaching patches for all supported versions - would be really good if we could squeeze this into 1.9Stable as well. - thanks!

Show

Dan Marsden added a comment - 11/Apr/12 5:13 PM attaching patches for all supported versions - would be really good if we could squeeze this into 1.9Stable as well. - thanks!

Hide

Permalink

Dan Poltawski added a comment - 12/Apr/12 10:11 AM

Looks good to me

Show

Dan Poltawski added a comment - 12/Apr/12 10:11 AM Looks good to me

Hide

Permalink

Dan Poltawski added a comment - 12/Apr/12 11:31 PM

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Show

Dan Poltawski added a comment - 12/Apr/12 11:31 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Hide

Permalink

Sam Hemelryk added a comment - 16/Apr/12 8:24 AM

Thanks Dan, this has been integrated now

Show

Sam Hemelryk added a comment - 16/Apr/12 8:24 AM Thanks Dan, this has been integrated now

Hide

Permalink

Ankit Agarwal added a comment - 18/Apr/12 10:01 AM - edited

Hi guys,
Tested this on ff/chromium/opera/chrome
Working as expected
But I noticed a couple of unrelated issues:-

Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/mod/scorm/locallib.php on line 538
Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/mod/scorm/player.php on line 173
Even if the setting "Allow resize of window is unchecked", still on all three browsers I was able to resize the window...I know this is browser dependent thing..but it still those are the major browsers used..am I missing something here?

Passing this test as the issues are completely un-related.
Thanks

Show

Ankit Agarwal added a comment - 18/Apr/12 10:01 AM - edited Hi guys, Tested this on ff/chromium/opera/chrome Working as expected But I noticed a couple of unrelated issues:- Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/mod/scorm/locallib.php on line 538 Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/mod/scorm/player.php on line 173 Even if the setting "Allow resize of window is unchecked", still on all three browsers I was able to resize the window...I know this is browser dependent thing..but it still those are the major browsers used..am I missing something here? Passing this test as the issues are completely un-related. Thanks

Hide

Permalink

Dan Marsden added a comment - 18/Apr/12 2:49 PM

thanks - feel free to create a new tracker issue (if there isn't one already for PHP 5.4) to fix those - I'd be surprised if they were the only SCORM ones that need fixing!

the allow resize stuff dates back to Moodle 1.5 when some of the browsers still allowed us to make those changes. - the title of the options should state "(Prevented by some browsers)" but as the browsers that do support it aren't supported by Moodle anymore my +1 to remove any unsupported ones from master.

Show

Dan Marsden added a comment - 18/Apr/12 2:49 PM thanks - feel free to create a new tracker issue (if there isn't one already for PHP 5.4) to fix those - I'd be surprised if they were the only SCORM ones that need fixing! the allow resize stuff dates back to Moodle 1.5 when some of the browsers still allowed us to make those changes. - the title of the options should state "(Prevented by some browsers)" but as the browsers that do support it aren't supported by Moodle anymore my +1 to remove any unsupported ones from master.

Hide

Permalink

Ankit Agarwal added a comment - 18/Apr/12 3:11 PM

Thanks Dan,
Created ~~MDL-32506~~ and ~~MDL-32507~~ for the same.
Thanks

Show

Ankit Agarwal added a comment - 18/Apr/12 3:11 PM Thanks Dan, Created MDL-32506 and MDL-32507 for the same. Thanks

Hide

Permalink

Dan Poltawski added a comment - 19/Apr/12 9:55 PM

Bonza mate!

Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby.

Hooroo

Show

Dan Poltawski added a comment - 19/Apr/12 9:55 PM Bonza mate! Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby. Hooroo

People

Assignee:

Dan Marsden

Reporter:

Harshal Mundiwale

Peer reviewer:

Dan Poltawski

Integrator:

Sam Hemelryk

Tester:

Ankit Agarwal

Participants:

Ankit Agarwal, Dan Marsden, Dan Poltawski, Harshal Mundiwale, Sam Hemelryk

Votes:

2 Vote for this issue

Watchers:

3 Start watching this issue

Dates

Created:

10/Apr/12 11:43 PM

Updated:

09/Aug/12 4:02 PM

Resolved:

19/Apr/12 9:55 PM

Fix Release Date:

14/May/12