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

FEAT: http-equiv meta tags #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ankostis
Copy link

That is a quick'n dirty implementation to output <meta http-equiv="..."> tags.
The above html element gets produces only for those tags in smtgTagsProperties that exist also in the new smtgMetaHttpEquivs GLOBALS array.

  • There is no documentation, apart from this description here.
  • The existing structure of the projects has not been honored - only the src/OutputPageHtmlTagsInserter.php file was modified.
  • It is provided just as a starting base.

@ankostis
Copy link
Author

ankostis commented Dec 12, 2018

Forgot to mention that i needed this patch to add a Content-Disposition header (and set the downloaded filename of some page).
Which it didn't work on Chrome (only Firefox honored it, but still added an .html extension).
In the end i found that {{DISPLAYTITLE}} achieves the same thing, much better.

Also noting that it this extension doesn't nothing for pages with content-type: text/plain (makes sense).

Copy link
Member

@kghbln kghbln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README file needs to be extended, too. I am not sure if the existing integration tests also cover this new parameter.


$comment = '';

if ( !$this->metaPropertyMarkup ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we need this here too, since this HTML comment is already added via the following lines

if ( !$this->metaPropertyMarkup ) {
$comment .= '<!-- Semantic MetaTags -->' . "\n";
$this->metaPropertyMarkup = true;
}

@@ -140,4 +144,30 @@ private function reqMetaPropertyMarkup( $tag ) {
return false;
}

private function reqMetaHttpEquivs( $tag ) {

// If a tag contains a `og:` such as `og:title` it is expected to be a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be adapted. This is the identical comment copied from the following lines:

// If a tag contains a `og:` such as `og:title` it is expected to be a
// OpenGraph protocol tag along with other prefixes maintained in
// $GLOBALS['smtgMetaPropertyPrefixes']

@kghbln
Copy link
Member

kghbln commented Dec 15, 2018

@ankostis Thanks a lot for submitting a pull request. I have commented while doing a first check. It will be great if @JeroenDeDauw could make some comments form a programmers perspective, too.

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.

2 participants