Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add ability to override meta tag in page_theme.php by making it conditional in header_required.php #4310

Closed
martbase opened this issue Sep 1, 2016 · 7 comments

Comments

@martbase
Copy link

martbase commented Sep 1, 2016

I follow bootstrap's recommendation for starting a basic template (see code below) when developing my themes. This results in a theme that doesn't pass w3c validation with the following error:-

A document must not include both a meta element with an http-equiv attribute whose value is content-type, and a meta element with a charset attribute.

My header.php already includes <meta charset="utf-8"> as the first tag in head but concrete5 also generates <meta http-equiv="content-type" content="text/html; charset=UTF-8"/> hence the duplication.

Since we can already conditionally load assets for IE in page_theme with requireAsset('javascript-conditional', 'html5-shiv'), it would also be nice to adjust concrete5's header_required output somehow without having to override header_required itself as its not recommended.

The option doesn't have to be configured from page_theme, any other solution would work just as well.

Here is bootstrap's basic template which I base my header.php on

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <!-- The above 3 meta tags *must* come first in the head; any other head content must come *after* these tags -->
    <title>Bootstrap 101 Template</title>

    <!-- Bootstrap -->

    <!-- HTML5 shim and Respond.js for IE8 support of HTML5 elements and media queries -->
    <!-- WARNING: Respond.js doesn't work if you view the page via file:// -->
    <!--[if lt IE 9]>
      <script src="https://oss.maxcdn.com/html5shiv/3.7.3/html5shiv.min.js"></script>
      <script src="https://oss.maxcdn.com/respond/1.4.2/respond.min.js"></script>
    <![endif]-->
  </head>
  <body>
    <h1>Hello, world!</h1>

  </body>
</html>

Martin

@mlocati
Copy link
Contributor

mlocati commented Sep 1, 2016

Well, the fastest solution would be that you remove your charset meta tag and use the one provided by concrete5.

BTW, since version 5.7.5.9 you have also another solution: add this code to your application/bootstrap/app.php file:

Events::addListener('on_header_required_ready', function($event) {
    $metaTags = $event->getArgument('metaTags');
    unset($metaTags['charset']);
    $event->setArgument('metaTags', $metaTags);
});

@martbase
Copy link
Author

martbase commented Sep 2, 2016

I like the second approach as it allows customization which is better in my opinion. Can the same be accomplished through package on_start()?

@mlocati
Copy link
Contributor

mlocati commented Sep 2, 2016

Sure!

@martbase
Copy link
Author

martbase commented Sep 2, 2016

Nice. Let me give it a shot.

@aembler
Copy link
Member

aembler commented Sep 2, 2016

Aren't they essentially the same?

http://stackoverflow.com/questions/4696499/meta-charset-utf-8-vs-meta-http-equiv-content-type

Couldn't you just remove the meta charset tag from the bootstrap chop since concrete5 already provides a content type? It's not like you don't have to remove other tags from the header (like title, meta description, etc...)

@martbase
Copy link
Author

martbase commented Sep 2, 2016

Thanks for the link (useful). I prefer the new shorter modern format that was introduced with HTML5, and maybe the old style will become deprecated in future.
Concrete5 allows me to customize templates for blocks and pages so why not for the HTML head section.

An extra benefit for the request is the ability to configure the output of header_required.

My header.php start's as below (with these three meta tags appearing first)

<?php defined('C5_EXECUTE') or die("Access Denied.");?>
<!DOCTYPE html>
<html lang="<?php echo Localization::activeLanguage()?>">

<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0" />

<?php Loader::element('header_required', array('pageTitle' => $pageTitle));?> 

@aembler
Copy link
Member

aembler commented Sep 4, 2016

Since it sounds like the event will work for this use case, I'm going to close this issue.

@aembler aembler closed this as completed Sep 4, 2016
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

No branches or pull requests

3 participants