Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow registering scriptFiles as async #2093

Closed
wants to merge 1 commit into from

4 participants

@fiesh

This fixes #1724, it adds the ability to add 'async="async"' to

@klimov-paul klimov-paul commented on the diff
framework/web/helpers/CHtml.php
((5 lines not shown))
* @return string the enclosed JavaScript
*/
- public static function script($text)
+ public static function script($text,$async=false)
@klimov-paul Collaborator

Instead of adding boolean field, it would be better to pass array "$htmlOptions", which should be used to compose a HTML tag.
See "CHtml::tag()" for ispiration and possible solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@klimov-paul klimov-paul commented on the diff
framework/web/CClientScript.php
@@ -56,6 +56,16 @@ class CClientScript extends CApplicationComponent
*/
public $scriptMap=array();
/**
+ * @var array the list of scripts that should always be included asynchronously.
+ * This allows specifying scripts that will be included with 'async="async"' set even if the call to {@link registerClientScript} does not specify it.
+ * This is particularly useful for scripts that appear in {@link $scriptMap}.
+ *
+ * If a script is registered as asynchronous, its name is appended to this array.
+ *
+ * @since 1.1.14
+ */
+ public $asyncScripts=array();
@klimov-paul Collaborator

It whould be better to modify "CClentScript::registerScriptFile()" adding third parameter for the additional HTML options.

@fiesh
fiesh added a note

That would make it really hard to have files register async that are registered by something you don't control. With this current option, you can just have a scriptMap like behavior that is much easier for production use.

@klimov-paul Collaborator

And what will you do for the "defer" option of script files?
Should we add a new field at "CClientScript" for each possible variations of script file link?

@fiesh
fiesh added a note

Nothing because defer does not really seem to be used. Apart from that, the script tag does not really support any useful "possible variations", i.e., attributes: http://www.w3schools.com/tags/tag_script.asp. But whatever, I'll close the pull request.

@klimov-paul Collaborator

The issue (#1724) is valid.
As for the possible solution, I would like to hear what @yiisoft think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fiesh

Just async is not valid XHTML

@fiesh fiesh closed this
@samdark samdark reopened this
@klimov-paul
Collaborator

Competitor pull request has been opened at #2172

@samdark samdark closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 9, 2013
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 8 deletions.
  1. +14 −4 framework/web/CClientScript.php
  2. +6 −4 framework/web/helpers/CHtml.php
View
18 framework/web/CClientScript.php
@@ -56,6 +56,16 @@ class CClientScript extends CApplicationComponent
*/
public $scriptMap=array();
/**
+ * @var array the list of scripts that should always be included asynchronously.
+ * This allows specifying scripts that will be included with 'async="async"' set even if the call to {@link registerClientScript} does not specify it.
+ * This is particularly useful for scripts that appear in {@link $scriptMap}.
+ *
+ * If a script is registered as asynchronous, its name is appended to this array.
+ *
+ * @since 1.1.14
+ */
+ public $asyncScripts=array();
@klimov-paul Collaborator

It whould be better to modify "CClentScript::registerScriptFile()" adding third parameter for the additional HTML options.

@fiesh
fiesh added a note

That would make it really hard to have files register async that are registered by something you don't control. With this current option, you can just have a scriptMap like behavior that is much easier for production use.

@klimov-paul Collaborator

And what will you do for the "defer" option of script files?
Should we add a new field at "CClientScript" for each possible variations of script file link?

@fiesh
fiesh added a note

Nothing because defer does not really seem to be used. Apart from that, the script tag does not really support any useful "possible variations", i.e., attributes: http://www.w3schools.com/tags/tag_script.asp. But whatever, I'll close the pull request.

@klimov-paul Collaborator

The issue (#1724) is valid.
As for the possible solution, I would like to hear what @yiisoft think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ /**
* @var array list of custom script packages (name=>package spec).
* This property keeps a list of named script packages, each of which can contain
* a set of CSS and/or JavaScript script files, and their dependent package names.
@@ -362,7 +372,7 @@ public function renderHead(&$output)
if(isset($this->scriptFiles[self::POS_HEAD]))
{
foreach($this->scriptFiles[self::POS_HEAD] as $scriptFile)
- $html.=CHtml::scriptFile($scriptFile)."\n";
+ $html.=CHtml::scriptFile($scriptFile,(in_array($scriptFile,$this->asyncScripts)?true:false))."\n";
}
if(isset($this->scripts[self::POS_HEAD]))
@@ -390,7 +400,7 @@ public function renderBodyBegin(&$output)
if(isset($this->scriptFiles[self::POS_BEGIN]))
{
foreach($this->scriptFiles[self::POS_BEGIN] as $scriptFile)
- $html.=CHtml::scriptFile($scriptFile)."\n";
+ $html.=CHtml::scriptFile($scriptFile,(in_array($scriptFile,$this->asyncScripts)?true:false))."\n";
}
if(isset($this->scripts[self::POS_BEGIN]))
$html.=CHtml::script(implode("\n",$this->scripts[self::POS_BEGIN]))."\n";
@@ -422,7 +432,7 @@ public function renderBodyEnd(&$output)
if(isset($this->scriptFiles[self::POS_END]))
{
foreach($this->scriptFiles[self::POS_END] as $scriptFile)
- $html.=CHtml::scriptFile($scriptFile)."\n";
+ $html.=CHtml::scriptFile($scriptFile,(in_array($scriptFile,$this->asyncScripts)?true:false))."\n";
}
$scripts=isset($this->scripts[self::POS_END]) ? $this->scripts[self::POS_END] : array();
if(isset($this->scripts[self::POS_READY]))
@@ -771,4 +781,4 @@ public function addPackage($name,$definition)
$this->packages[$name]=$definition;
return $this;
}
-}
+}
View
10 framework/web/helpers/CHtml.php
@@ -276,21 +276,23 @@ public static function cssFile($url,$media='')
/**
* Encloses the given JavaScript within a script tag.
* @param string $text the JavaScript to be enclosed
+ * @param boolean $async whether the script should be executed asynchronously, defaults to false. (@since 1.1.14)
* @return string the enclosed JavaScript
*/
- public static function script($text)
+ public static function script($text,$async=false)
@klimov-paul Collaborator

Instead of adding boolean field, it would be better to pass array "$htmlOptions", which should be used to compose a HTML tag.
See "CHtml::tag()" for ispiration and possible solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
{
- return "<script type=\"text/javascript\">\n/*<![CDATA[*/\n{$text}\n/*]]>*/\n</script>";
+ return '<script type="text/javascript"'.($async?' async="async"':'').">\n/*<![CDATA[*/\n{$text}\n/*]]>*/\n</script>";
}
/**
* Includes a JavaScript file.
* @param string $url URL for the JavaScript file
+ * @param boolean $async whether the script should be included asynchronously, defaults to false. (@since 1.1.14)
* @return string the JavaScript file tag
*/
- public static function scriptFile($url)
+ public static function scriptFile($url,$async=false)
{
- return '<script type="text/javascript" src="'.self::encode($url).'"></script>';
+ return '<script type="text/javascript"'.($async?' async="async"':'').' src="'.self::encode($url).'"></script>';
}
/**
Something went wrong with that request. Please try again.