Skip to content

Comments

Improve inline js/css type rendering in html5#9843

Merged
rdeutz merged 2 commits intojoomla:stagingfrom
andrepereiradasilva:patch-12
May 2, 2016
Merged

Improve inline js/css type rendering in html5#9843
rdeutz merged 2 commits intojoomla:stagingfrom
andrepereiradasilva:patch-12

Conversation

@andrepereiradasilva
Copy link
Contributor

Pull Request for Improvement.

Summary of Changes

This PR improves inline js/css rendering in HTML5.

In HTML5 there are default type for:

  • script tag: text/javascript
  • style tag: text/css

So they are not needing when the document is rendered in HTML5.

Joomla already removes them in HTML5 for external js/css files, but not for inline js/css.
This PR improves that.

Testing Instructions

  • Use latest staging
  • Go to protostar homepage and view source. You will get some
<script type="text/javascript">[...]</script>
  • Apply patch
  • You willl now get
<script>[...]</script>

To test the style tag you can add to your protostar index.php file, for instance:

$doc->addStyleDeclaration('p {margin:0}');

That should be rendered, after the patch. as

<style>
p {margin:0}
</style>

Also check code difference.

@ghost
Copy link

ghost commented Apr 11, 2016

I have tested this item ✅ successfully on 20a41a8

Tested script and style tag on protostar, both working als described.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9843.

@waader
Copy link
Contributor

waader commented Apr 15, 2016

In protostar this block is rendered unchanged:
<style type="text/css"> h1,h2,h3,h4,h5,h6,.site-title{ font-family: 'Open Sans', sans-serif; } </style>

Also in Beez there is this unchanged block:

<script type="text/javascript"> var big = '72%'; var small = '53%'; var bildauf = '/joomla36/templates/beez3/images/plus.png'; var bildzu = '/joomla36/templates/beez3/images/minus.png'; var rightopen = 'Open info'; var rightclose = 'Close info'; var altopen = 'is open'; var altclose = 'is closed'; </script>

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Apr 15, 2016

@waader that's true. That happens because someone didn't use the Joomla API to load the style and javascript.
See, for instance, https://github.com/joomla/joomla-cms/blob/staging/templates/protostar/index.php#L98-L103 for protostar.

But that as nothing to do with this PR does.

@waader
Copy link
Contributor

waader commented Apr 15, 2016

I have tested this item ✅ successfully on 20a41a8

Thanks for the explanation!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9843.

@andrepereiradasilva
Copy link
Contributor Author

@brianteeman RTC?

@brianteeman
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9843.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 17, 2016
@brianteeman brianteeman added this to the Joomla! 3.6.0 milestone Apr 17, 2016
@rdeutz rdeutz merged commit 6baba4c into joomla:staging May 2, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 2, 2016
@andrepereiradasilva andrepereiradasilva deleted the patch-12 branch May 2, 2016 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants