Hi,
So, I thought I would try the 'Combine CSS files by using media queries' option in the Configuration section. The help text on that item says 'Use cores grouping logic needs to be unchecked in order for this to work.' So, I've turned that off which now breaks javascript and I'm getting errors in the console stating 'Uncaught ReferenceError: MQ is not defined (anonymous function)'. I'm noticing this happens regardless of whether the 'Combine CSS files by using media queries' option is checked or not. You should be able to reproduce the errors simply by unchecking the 'Use cores grouping logic' option.
Furthermore, if you have both options turned on, from a cursory glance, it doesn't look like it causes any problems (contrary to the help text warning). I'm wondering if the help text is just incorrect and that the 'Use cores grouping logic' should just always be on.
Anyway, I wanted to bring this to your attention.
Erik.
Comment | File | Size | Author |
---|---|---|---|
#27 | labjs-1990358-fix-for-advagg.patch | 2.68 KB | btopro |
#24 | labjs-1990358-fix-for-advagg.patch | 2.68 KB | mikeytown2 |
#23 | advagg-1990358-23-add-in-moveable-key.patch | 1.55 KB | mikeytown2 |
Comments
Comment #1
erikhopp CreditAttribution: erikhopp commentedIt looks like this undefined MQ issue comes from the media_queries.js file in the Adaptive Theme subtheme. The MQ variable is produced in the at_core/scripts/onmediaquery.js file so, perhaps turning off the 'Use core grouping logic' option is somehow excluding that file?
Comment #2
mikeytown2 CreditAttribution: mikeytown2 commentedThanks for pointing me in the right direction :)
Comment #3
mikeytown2 CreditAttribution: mikeytown2 commentedIt's own js_alter could be messing advagg up
http://drupalcode.org/project/adaptivetheme.git/blob/45bd4bdb3073d722040...
See #1946218: test with other modules & themes
Comment #4
mikeytown2 CreditAttribution: mikeytown2 commentedI've added in debug mode #1996802: have advagg-debug=1 output debug info to watchdog
If i could get the output from watchdog that would be helpful.
path/to/bug?advagg-debug=1
Comment #5
mikeytown2 CreditAttribution: mikeytown2 commentedComment #6
mikeytown2 CreditAttribution: mikeytown2 commentedComment #7
mikeytown2 CreditAttribution: mikeytown2 commented99% sure this issue was fixed by this: #2039297: External script order is not strictly being kept.
Comment #9
erikhopp CreditAttribution: erikhopp commentedHi,
It took me a while to get back in investigating this, but I have and I'm still seeing largely the same issue - but some of the details are a bit different - and I think I've figured out a potential cause.
I'm still seeing the following error in console: 'Uncaught ReferenceError: MQ is not defined' (in media_queries.js - in my AdaptiveTheme subtheme) and I'm also seeing 'Uncaught ReferenceError: jQuery is not defined' in the Highcharts module /js/highcharts.js file (7.x-2.x-dev version) - which is breaking Highcharts. But it is no longer breaking javascript functionality completely.
I'm noticing that, for both of these files, they are designated to be loaded in the footer of the document (scope = 'footer') and when I switch them to the header, the errors go away. In the debugging export, it is looking like the 'footer' grouping is loading before the 'header' grouping of js files (perhaps it is loading alphabetically?) - which may be causing problems when variables required for the files that are to be loaded in the footer are not yet defined.
Hopefully my investigative work is helpful. I'd appreciate hearing what you think about this.
Thanks!
Erik.
Comment #10
erikhopp CreditAttribution: erikhopp commentedRetitling to reflect latest debugging clues.
Comment #11
erikhopp CreditAttribution: erikhopp commentedGrammar.
Comment #12
mikeytown2 CreditAttribution: mikeytown2 commentedIn terms of highcharts do you have this file
sites/all/libraries/highcharts/js/highcharts.js
where highcharts.js is version 2.3.5? Example file as highcharts.com only lists the 3.x branch for download: http://cdnjs.cloudflare.com/ajax/libs/highcharts/2.3.5/highcharts.jsThe readme doesn't explain that you need to copy the 2.3.5 version into that directory. Once I did this the JS errors went away.
I can't repo the footer loading before the header, I'll try out AdaptiveTheme to see if I can repo this.
Comment #13
mikeytown2 CreditAttribution: mikeytown2 commentedUsing AdaptiveTheme base subtheme 7.x-3.1 and turning "Responsive JavaScript beta" under Polyfills on
admin/appearance/settings/adaptivetheme_subtheme
I can't get the MQ is not defined error to happen.I need more details on how to repo the reported bug.
Comment #14
erikhopp CreditAttribution: erikhopp commentedHi mikeytown2,
I do indeed have a highcharts library loading, but I'm using a later version - 3.0.7.
I'm not sure what to say other than that when javascript files seem to be scoped to load in the footer, there are JS errors showing up when it appears that they *think* the header JS files are loading first and require variables like jQuery and MQ to be set already - but are not because it looks like the JS files scoped for the footer are loading before ones scoped for the header.
The following code changes fixed the two issues cited above and now produce no errors when AdvAgg is enabled:
Highcharts
And the AT theme file
Does that help?
Erik
Comment #15
mikeytown2 CreditAttribution: mikeytown2 commentedIf you go to
admin/config/development/performance/advagg/info
and under the Hooks And Variables Used In Hash section what is listed in there?Comment #16
erikhopp CreditAttribution: erikhopp commentedComment #17
mikeytown2 CreditAttribution: mikeytown2 commentedOn that same page what is listed under "Modules implementing AdvAgg CSS/JS hooks"?
Comment #18
erikhopp CreditAttribution: erikhopp commentedComment #19
mikeytown2 CreditAttribution: mikeytown2 commentedAfter installing labjs I am able to repo the reported issue with "MQ is not defined".
Comment #20
mikeytown2 CreditAttribution: mikeytown2 commentedLooks like LABjs is not wrapping the footer code in labjs markup. Will debug.
Comment #21
mikeytown2 CreditAttribution: mikeytown2 commentedLooks like a change in this #2122237: Compatibility problem with Media module's media browser popup broke LABjs. Issue is the header JS gets processed last instead of first (advagg follows what core does now). It appears that I'll have to roll a patch for LABjs in order to fix this. Will move this issue over there once I have the patch ready to go.
Comment #22
erikhopp CreditAttribution: erikhopp commentedAwesome. Thanks a bunch!
Comment #23
mikeytown2 CreditAttribution: mikeytown2 commentedThe following patch has been committed to AdvAgg's mod submodule. Up next is a patch for LABjs.
Comment #24
mikeytown2 CreditAttribution: mikeytown2 commentedComment #25
erikhopp CreditAttribution: erikhopp commentedThe patches seem to fix the issue. Console is free of errors! Thanks! Erik.
Comment #26
erikhopp CreditAttribution: erikhopp commentedHi! I just wanted to give this a push and remind that the current patch fixes the bug and is ready to be committed, in my opinion. Erik.
Comment #27
btopro CreditAttribution: btopro commentedproperty had a misspelling. What was committed to advagg said the property is called movable and the original patch was calling for moveable. I'm experiencing wonkiness in advagg but I can't pinpoint what yet (it does appear related to this module though).
Comment #28
jcisio CreditAttribution: jcisio commentedIt was renamed to movable in http://cgit.drupalcode.org/advagg/commit/advagg_mod/advagg_mod.module?id....
Thanks for the patch. Committed and pushed.