Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Render client scripts after all meta tags #1866

Merged
merged 5 commits into from

8 participants

@horechek

Now scripts are rendered before "<title>"(if POS_HEAD). It's not good for SEO. It would be better to render them before "</head>"

@Maxsh

I think it necessary as well

@mdomba
Collaborator

Please explain how this would not be good for SEO?

@Maxsh

For example, we need this behavior for register Google Analytics code
https://support.google.com/analytics/bin/answer.py?hl=en&answer=1008080
Once you find the code snippet, copy and paste it into your web page, just before the closing tag*.

@samdark
Owner

@Maxsh can't you just copy-paste it to main.php layout template?

@Maxsh

@samdark This is not good way for us. We use the module SEO and one can be enabled or disabled in the runtime. It would be great if the GA code can be registered by CClientScript.

@horechek

<title> tag is the place to put page’s best keywords. In fact, the value of the words used in the tag are so potent that the search engines can deem the first word the most valuable word used, the second word next most valuable, and so on in descending order.

@horechek

Furthermore, most of the templates stick to these standards https://github.com/h5bp/html5-boilerplate/blob/master/index.html

@mdomba
Collaborator

Yes, the title tag holds the important keywords, but the position of the title tag is not important as long as it's in the HEAD section.

We already had a similar issue #1388

@mdomba
Collaborator

In addition we cannot just move the above TITLE to above HEAD as that would break BC.

@Maxsh

Wherefore scripts are rendered before tag title in Yii?
If I paste GA code just before </head> I will get the scripts before and after title.
I propose render all scripts just before </head> and ignore tag title

@December24

In addition to SEO considerations, executing scripts may require time, and until the scripts are executed the user sees nothing but a blank page. Seeing at least a title would make the user understand that the page is being loaded, and decreases risks of abandoning the page.

@samdark
Owner

@Maxsh actually it doesn't matter if Google's script is before title or after. The idea is that it's there as soon as possible so it will be triggered even when page isn't fully loaded. Personally I'm putting GA scripts just before closing <body /> not to slow page loading down.

@December24 your case is the only valid so far :)

@samdark
Owner

I guess we just need to check if </head> is required by all HTML specs. If it is, it's safe to remove checking for anything else.

@Maxsh

@samdark "I'm putting GA scripts just before closing <body /> not to slow page loading down."
of course since the GA code is asynchronous script

About scripts before and after tag title, I mean that if I register some scripts and just paste GA code before closing tag head I will get the following result:

  • rendered tags script
  • after that tag title
  • after that tag script again

and It would be great if we will have one placeholder to render script (before closing head tag :) )

(of course code GA independs on tag title :) )

@Maxsh

Unfortunately, closing tag head is optional :(
http://www.w3.org/TR/REC-html40/index/elements.html

But if we can not be based on tag head, maybe we can though render scripts after tag title :)

@samdark
Owner

@Maxsh that would work and is backwards compatible.

@samdark
Owner

@horechek can you adjust your pull request accordingly?

@horechek horechek Update framework/web/CClientScript.php
Changed renderHead() method for render clients scripts after </title>
8433a3b
@horechek

sorry

@horechek horechek Update framework/web/CClientScript.php
Changed renderHead() method for render clients scripts after </title>
28c198a
@samdark
Owner

I think it's better not remove head part.

@December24

In my experience SEO specialists often ask for the following structure:
< head >
  charset
  title
  everything else
  GA code
< /head >

I understand it is not required by HTML standard but it is required by 90% of marketers. I think it would make the framework more friendly if the suggested tweak is applied.

@horechek horechek Update framework/web/CClientScript.php
Сompromise solution. If we has a <head> we use him, else we use <title>
802939e
@horechek

Сompromise solution

@Maxsh

As for me, this solution is more flexible since it resolves the my issue with division GA code and title tag and I can implement structure like this
https://github.com/h5bp/html5-boilerplate/blob/master/index.html

@December24

I like this one

@horechek

guys?

@samdark
Owner

Will be back to this one after 1.1.13 release.

@horechek

let's continue

@samdark
Owner

I just need to some testing before I can merge it.

@mdomba
Collaborator

As I stated before changing from before title to before head can break existing code as currently someone can include a custom CSS before HEAD to be sure that his CSS is the last one so his rules would be the most effective.

IMO we can safely change from before TITLE to after TITLE... would that solve all your problems?

@jacmoe

Why not a dead simple solution?

A comment where you want Yii to put the stuff:

<!--- yii_scripts begin -->
<!-- yii_scripts end -->

Actually, I think that it could be config options, the comment text?

That would be so much simpler, IMO.

This is even better because sometimes we need to include custom css before or after the clientscript inserted css.

@klimov-paul
Collaborator

Actually, it would be the best, if we simply invoke "CClientScript::render()" explicitely into the layout file, instead of parsing the entire page output.

<!DOCTYPE ...>
<head>
<?php echo Yii::app()->clientScript()->renderHead(); ?>
</head>
@samdark samdark merged commit 072bebb into yiisoft:master
@samdark samdark referenced this pull request from a commit
@samdark samdark added changelog line for #1866 c269a68
@samdark
Owner

Merged. Thanks for working on it.

@mdomba
Collaborator

As I commented above this could break BC...

Before the CSS where rendered before TITLE so if someone had inserted manually (ex. in the main.php) a CSS file after the title that file would replace all previous CSS rules...

Now with this change the CSS is rendered before HEAD so that would be the last CSS and the one inserted after the TITLE becomes first not last as before so it cannot override the CSS rules anymore.

@samdark samdark referenced this pull request from a commit
@samdark samdark Revert "added changelog line for #1866"
This reverts commit c269a68.
242b8d7
@samdark
Owner
@samdark
Owner

Yes, you're right. I've reverted changes and I don't think we can apply any w/o breaking BC. So the final decision is that we won't address this issue in 1.1.

@mdomba
Collaborator

As we commented somewhere above... a possible solution would be to change from before TITLE to after TITLE, this way we solve this issue and no problem with BC as the manual one would still be after those inserted...

@andsigno82

i just wrote a possible solution and i'm just creating a pull request for that

@andsigno82 andsigno82 referenced this pull request from a commit in andsigno82/yii
@andsigno82 andsigno82 Updates /framework/web/CClientScript.php
references #1866
7f2e02a
@andsigno82 andsigno82 referenced this pull request from a commit in andsigno82/yii
@andsigno82 andsigno82 Updates /framework/web/CClientScript.php
references #1866
6acf6ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 18, 2012
  1. @horechek
  2. @horechek

    Update framework/web/CClientScript.php

    horechek authored
    Changed renderHead() method for render clients scripts after </title>
  3. @horechek

    Update framework/web/CClientScript.php

    horechek authored
    Changed renderHead() method for render clients scripts after </title>
  4. @horechek

    Update framework/web/CClientScript.php

    horechek authored
    Сompromise solution. If we has a <head> we use him, else we use <title>
Commits on Feb 26, 2013
  1. @horechek

    Update framework/web/CClientScript.php

    horechek authored
    fixed error
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 2 deletions.
  1. +5 −2 framework/web/CClientScript.php
View
7 framework/web/CClientScript.php
@@ -372,7 +372,10 @@ public function renderHead(&$output)
if($html!=='')
{
$count=0;
- $output=preg_replace('/(<title\b[^>]*>|<\\/head\s*>)/is','<###head###>$1',$output,1,$count);
+ $output=preg_replace('/(<\\/head\s*>)/is','<###head###>$1',$output,1,$count);
+ if(!$count)
+ $output=preg_replace('/(<\\/title\s*>)/is','$1<###head###>',$output,1,$count);
+
if($count)
$output=str_replace('<###head###>',$html,$output);
else
@@ -771,4 +774,4 @@ public function addPackage($name,$definition)
$this->packages[$name]=$definition;
return $this;
}
-}
+}
Something went wrong with that request. Please try again.