Incompatible with Safari [#1128944]

After spending too much time on this I realized that #1127530: Use of a reserved word and this issue are for 6.x-3.x-dev and issue #1221668: Breaks a lot of functionality when Aggregate JavaScript is on is for 7.x-3.x

This issue came from #1127530: Use of a reserved word, which turned out to be two parts.
Apparently, even after that patch, AMH still does not work with Safari.

Comment	File	Size	Author
#7	admin_menu_dropdown_fix_javascript_1128944_6.patch	1.2 KB	izmeez
#5	admin_menu_dropdown_1128944_5.patch	1.38 KB	izmeez
#2	admin_menu_dropdown_1128944_2.patch	1.68 KB	Josh The Geek

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Josh The Geek CreditAttribution: Josh The Geek commented 15 April 2011 at 23:37

The problem is the defer attribute: I use the defer attribute of the script tag to delay the running of amh's scripts until the config variable is set up. According to http://www.w3schools.com/tags/att_script_defer.asp , IE is the only browser that supports this tag. (Apparently, it works in Firefox and Chrome, too.)

Safari does not know what the attribute means, so it gets skipped. The scripts are run before the config variable is loaded, causing a 'TypeError: Result of expression 'Drupal.settings.admin_menu_dropdown' [undefined] is not an object.'

The solution is to wrap the code in jQuery(document).ready(function(){}). Patch coming shortly.

Screenshot of Safari debugger: https://skitch.com/joshthegeek/r4q94/defer-attribute-used-by-admin-menu-...

Comment #2

Josh The Geek CreditAttribution: Josh The Geek commented 15 April 2011 at 23:46

Status:

Active

» Needs review

File	Size
admin_menu_dropdown_1128944_2.patch	1.68 KB

Patch.

Comment #3

ejohnson CreditAttribution: ejohnson commented 19 April 2011 at 16:26

Patch seems to fix it now in Safari.

Testing in Firefox 3 and IE8 as well.

Thanks.

Comment #4

distinctgrey CreditAttribution: distinctgrey commented 19 July 2011 at 14:36

After having applied the patches from both issues, Firebug still showed errors.

What you want is to execute the JS only after the config variable has been set up.
To do this correctly in Drupal, do not use document.ready(), but wrap the JS code as follows:

Drupal.behaviors.admin_menu_dropdown = function(context) {
	if (Drupal.settings.admin_menu_dropdown.onload) {
		$('#admin-menu').hide();
		$('body').addClass('adm_menu_hidden');
	}
	$(document).keypress(function(e) {
	  var unicode=e.keyCode ? e.keyCode : e.charCode;
	  if (String.fromCharCode(unicode) == Drupal.settings.admin_menu_dropdown.key) {
		$('#admin-menu').slideToggle('fast');
		$('body').toggleClass('adm_menu_hidden');
	  }
	});
}

This works for me, without any more errors showing up in Firebug.

Comment #5

izmeez CreditAttribution: izmeez commented 28 October 2018 at 15:30

Priority:	Critical	» Normal
Issue summary:	View changes

File	Size
admin_menu_dropdown_1128944_5.patch	1.38 KB

~~As a followup to @distinctgrey attached is the suggestion in comment #4 as a patch for review.~~

Ignore this patch, it is confusing 7.x-3.x with 6.x-3.x which is what this issue is about.

Comment #6

izmeez CreditAttribution: izmeez commented 28 October 2018 at 15:32

Version:

6.x-3.x-dev

» 7.x-3.x-dev

The patch I uploaded in #5 is actually for admin_menu_dropdown 7.x-3.x so I am changing the version.
I hope the suggestion in comment #4 was for 7.x-3.x but I have not tested with safari, not my usual environment.
1 will have to do some tests to see if this is even needed.

Removing comments as not relevant.