Heh, why can't you use any hook instead of regex and page source changing.

<!--[if lt IE 7 ]> <body id="homepage" class="ie ie6 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao">
<!-- Google Tag Manager -->
......................
<!-- End Google Tag Manager --> <![endif]-->
<!--[if IE 7 ]>    <body id="homepage" class="ie ie7 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if IE 8 ]>    <body id="homepage" class="ie ie8 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if IE 9 ]>    <body id="homepage" class="ie ie9 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if !IE]><!--> <body id="homepage" class="html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <!--<![endif]-->

As you can see in that specific case HTML is not valid. GTM appears between HEAD and BODY and <!-- <!-- construction is not allowed.

We have

<!--[if IE 7 ]>    <body id="homepage" class="ie ie7 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if IE 8 ]>    <body id="homepage" class="ie ie8 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if IE 9 ]>    <body id="homepage" class="ie ie9 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <![endif]-->
<!--[if !IE]><!--> <body id="homepage" class="html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"> <!--<![endif]-->

In our page template and the first one conditional comment
<!--[if lt IE 7 ]> <body id="homepage" class="ie ie6 html front not-logged-in one-sidebar sidebar-second page-node page-node- page-node-603 node-type-panel tao"><!--<![endif]-->

outside our HTML, it seems it comes from parent theme. But that doesn't matter at all i think.

CommentFileSizeAuthor
#3 google_tag-2364013-3.patch434 bytesJody Lynn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rantie86@gmail.com’s picture

Issue summary: View changes
rantie86@gmail.com’s picture

This fixe the issue:

//$children = preg_replace('@<body[^>]*>@', '$0' . $script, $children, 1);
$children = preg_replace('/!IE[a-z A-Z\S]*(body)[a-z A-Z\S]*(?:endif]-->)/', '$0' . $script, $children, 1);
Jody Lynn’s picture

Status: Active » Needs review
FileSize
434 bytes

This is a patch of rantie86's solution.

I found this issue because I was having a bug that made me unable to upload or remove files using the core file ajax widget. I suppose a specific combination of HTML and/or js led to this invalid HTML wreaking havoc on my site. It was not easy to track it down to this module.

I can confirm that the patch fixes my issue, but I'm not good enough with regex to confirm if the solution is otherwise good. I can definitely agree it's critical. It's possible that users of this module are having bugs and do not know this is the source.

solotandem’s picture

The fix you two like is specific to your use cases. If the conditional statements were arranged in a different order or included different tests, then your regex would fail. My questions to you (or anyone else reading this issue) are:

  • What is generating the conditional comments around the body tag? Your module or theme code, or a template file? Please identify the theme and paste a relevant snippet.
  • What alternatives are there to this HTML output? For example, it would seem that conditional CSS files would accomplish the same thing? And their use is not problematic in this regard. And would seem to be a preferable approach. (But enlighten me if you think otherwise.)
  • Give what I beleive is the uncertainty of trying to place content immediately after the body tag, would it not be more straightforward for you to override the hook_page_alter() additions by this module and then insert the Google Tag snippet where it makes sense for your themes?

@rantie86@gmail.com, if you know of a different hook that can be used with certainty to accomplish this insertion (and without regex), then please indicate such.

A regular expression was used because of the uncertainty of what might be in the page and how it might come together. To my knowledge, it is not a trivial task to place some content first on the page without knowledge of the theme. Again, I am open to suggestions on how to accomplish this.

Jody Lynn’s picture

Thanks solotandem. Looks like rantie's regex only served in my case to make sure I end up with no gtm script, rather than fixing it. Perhaps my issue is an unrelated bug with the gtm script.

My html around the beginning of body is:

</head>
<body class="html front logged-in no-sidebars page-node page-node- page-node-4346 node-type-section-overview homepage i18n-en page-panels" >
<!-- Google Tag Manager -->
<noscript><iframe src="//www.googletagmanager.com/ns.html?id=GTM-57RJQ5" height="0" width="0" style="display:none;visibility:hidden"></iframe></noscript>
<script type="text/javascript">(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0];var j=d.createElement(s);var dl=l!='dataLayer'?'&l='+l:'';j.src='//www.googletagmanager.com/gtm.js?id='+i+dl;j.type='text/javascript';j.async=true;f.parentNode.insertBefore(j,f);})(window,document,'script','dataLayer','GTM-57RJQ5');</script>
<!-- End Google Tag Manager -->
  <div class="search-mask"></div>
Jody Lynn’s picture

I found the real source of my issue: #2424749: Page paths setting is ineffective.

bartramos’s picture

Status: Needs review » Closed (duplicate)
Related issues: +#2638752: add configurable regexp

This issue has been sleeping for more than a year now. There's another issue that might be a solution to this case: #2638752-2: add configurable regexp so I will close this issue. If someone has a valid reason to reopen it, please be my guest :-)