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

Fixing two issues #54

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@azirer

azirer commented Jul 30, 2017

Related issue 1: #30
Related issue 1: #24

azirer added some commits Jul 30, 2017

Update abovethefold.class.php
Giving writing permission to the public is a security risk as discussed in  #30
Remove rel="abtf" because it breaks validation
Removed  rel=\"abtf\" and rel="abtf" because the HTML5 validator returns the following errors
1)Error: Attribute rel not allowed on element script at this point.
2)Error: Attribute rel not allowed on element style at this point.
@optimalisatie

This comment has been minimized.

Show comment
Hide comment
@optimalisatie

optimalisatie Jul 31, 2017

Owner

Hi @azirer,

Thanks a lot for providing the fix! The change of permissions has been applied in v2.7.12.

Regarding the use of rel="abtf". It is used as marker for plugins that extract inline CSS from the HTML such as Autoptimize. There is no indication that it breaks HTML5 validation.

/**
 * Skip CSS from Autoptimize minificaiton
 */
public function skip_css($excludeCSS) {
	$excludeCSS .= ',rel="abtf"';
	return $excludeCSS;
}

Do you have more information regarding the breaking of validation?

Owner

optimalisatie commented Jul 31, 2017

Hi @azirer,

Thanks a lot for providing the fix! The change of permissions has been applied in v2.7.12.

Regarding the use of rel="abtf". It is used as marker for plugins that extract inline CSS from the HTML such as Autoptimize. There is no indication that it breaks HTML5 validation.

/**
 * Skip CSS from Autoptimize minificaiton
 */
public function skip_css($excludeCSS) {
	$excludeCSS .= ',rel="abtf"';
	return $excludeCSS;
}

Do you have more information regarding the breaking of validation?

@azirer

This comment has been minimized.

Show comment
Hide comment
@azirer

azirer Jul 31, 2017

Hi @optimalisatie you should use https://validator.nu for html5 specifically validation.

See at https://html.spec.whatwg.org/multipage/scripting.html#the-script-element the global attributes that a script element can have.

However I get your point and I would suggest the use of custom data attributes https://www.w3.org/TR/html5/dom.html#embedding-custom-non-visible-data-with-the-data-%2a-attributes. Although I haven't tested them inside scrpt elements I'm pretty sure it will validate. Going to check tomorrow for sure as I'm on mobile right now

azirer commented Jul 31, 2017

Hi @optimalisatie you should use https://validator.nu for html5 specifically validation.

See at https://html.spec.whatwg.org/multipage/scripting.html#the-script-element the global attributes that a script element can have.

However I get your point and I would suggest the use of custom data attributes https://www.w3.org/TR/html5/dom.html#embedding-custom-non-visible-data-with-the-data-%2a-attributes. Although I haven't tested them inside scrpt elements I'm pretty sure it will validate. Going to check tomorrow for sure as I'm on mobile right now

@azirer

This comment has been minimized.

Show comment
Hide comment
@azirer

azirer Aug 8, 2017

Just tested data-rel="abtf" instead of rel="abtf" and it validates.

Be sure to read this ttps://rocketvalidator.com/articles/w3c-validator-is-legacy-long-live-validator-nu and perhaps forget all about validator.w3.org

What do you think @optimalisatie ?

azirer commented Aug 8, 2017

Just tested data-rel="abtf" instead of rel="abtf" and it validates.

Be sure to read this ttps://rocketvalidator.com/articles/w3c-validator-is-legacy-long-live-validator-nu and perhaps forget all about validator.w3.org

What do you think @optimalisatie ?

azirer added some commits Aug 8, 2017

Use data-rel="abtf" instead of rel="abtf"
to exclude scripts and styles from Autoptimize optimization
Related #54
@optimalisatie

This comment has been minimized.

Show comment
Hide comment
@optimalisatie

optimalisatie Aug 21, 2017

Owner

Hi @azirer,

Thanks again for the suggestions and research! The change of the attribute has been applied in v2.8.0.

Owner

optimalisatie commented Aug 21, 2017

Hi @azirer,

Thanks again for the suggestions and research! The change of the attribute has been applied in v2.8.0.

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