WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 3 months ago

Last modified 8 weeks ago

#42804 closed task (blessed) (fixed)

type is not required in HTML5

Reported by: sasiddiqui Owned by: SergeyBiryukov
Milestone: 5.3 Priority: low
Severity: normal Version:
Component: Script Loader Keywords: needs-testing has-patch commit
Focuses: Cc:
PR Number:

Description

type is no more required in style and script tags in HTML5. Now HTML Validator start throwing warnings on the script and style tags which contains the types.

Attaching a path which fixes the issues for inline and enqueued files (CSS and JS).

Attachments (8)

class.wp-scripts.php (13.7 KB) - added by sasiddiqui 2 years ago.
Fixed the type issue with enqueue and inline script tags
class.wp-styles.php (9.5 KB) - added by sasiddiqui 2 years ago.
Fixed the type issue with enqueue and inline style tags
class.wp-styles.2.php (9.5 KB) - added by sasiddiqui 2 years ago.
Fixed the type issue with enqueue and inline style tags updated
wp-includes.patch (13.6 KB) - added by sasiddiqui 2 years ago.
42804.patch (6.2 KB) - added by sasiddiqui 2 years ago.
42804-update1.patch (6.2 KB) - added by sasiddiqui 2 years ago.
42804.diff (23.1 KB) - added by SergeyBiryukov 3 months ago.
42804.2.diff (1.6 KB) - added by azaozz 3 months ago.

Download all attachments as: .zip

Change History (49)

@sasiddiqui
2 years ago

Fixed the type issue with enqueue and inline script tags

@sasiddiqui
2 years ago

Fixed the type issue with enqueue and inline style tags

#1 @knutsp
2 years ago

  • Type changed from defect (bug) to enhancement

Hello and welcome to track, sasiddiqui.

Thank your for your suggestion. I agree this would be a nice enhancement. When validating html these warnings comes up. WordPress could do better with making correct html.

Are you able to make a patch with just the changes you propose?

#2 @swissspidy
2 years ago

  • Component changed from General to Script Loader
  • Priority changed from normal to low
  • Version trunk deleted

Please have a look at https://make.wordpress.org/core/handbook/tutorials/working-with-patches/ in order to create patch files that can be more easily scanned. Uploading whole files is unfortunately not really helpful.

Also, just because type is not required anymore with HTML5, it doesn't mean every website or teme is using HTML5. In HTML4 and other doc types, the type attribute is required and will cause validation errors (not just warnings as in your example).

Just removing type might not the best solution. Maybe we need to use add_theme_support for this?

@sasiddiqui
2 years ago

Fixed the type issue with enqueue and inline style tags updated

#3 @sasiddiqui
2 years ago

@knutsp Thanks. @swissspidy Sorry about the improper attachments. Would create a patch and add it again.

Apology for the improper files.

#4 @sasiddiqui
2 years ago

  • Keywords has-patch needs-testing added

@knutsp @swissspidy Patch has been attached. Let me know if i have done any mistake in it.

@swissspidy As per your concern about the HTML 4 so, i am not sure but i think, mostly themes are working on HTML 5 specially the sites which are working on the latest WordPress Version. It's quite long time when HTML 5 had been introduced.

But, if you guys think that we still need to support the HTML 4 so, we can also add the filter for HTML 4 which doesn't remove the type attr from the script and style tags.

#5 @swissspidy
2 years ago

  • Keywords needs-patch added; has-patch removed

Patch has been attached. Let me know if i have done any mistake in it.

Much better, but the patch should only change the lines in question. Right now, it changes lots of the coding style throughout the files and the patch is very long and difficult to scan because of this. It's recommended not to touch irrelevant code sections.

For now and updated code you can use PHP Code Sniffer with the WP Coding Standards (https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards) to automatically correct coding standards mistakes. The WordPress Git repository contains a phpcs.xml.dist file to automatically configure PHPCS for that.

But, if you guys think that we still need to support the HTML 4 so, we can also add the filter for HTML 4 which doesn't remove the type attr from the script and style tags.

That's what I meant with add_theme_support. There's precedence for that. See https://developer.wordpress.org/reference/functions/add_theme_support/#html5

#6 follow-up: @knutsp
2 years ago

Something like add_theme_support( 'html5', [ 'script', 'css' ] );?

We already have add_theme_support( 'html5', [ 'comment-list', 'comment-form', 'search-form', 'gallery', 'caption' ] );

Could this be simplified to just add_theme_support( 'html5' ); to catch all?

Last edited 2 years ago by knutsp (previous) (diff)

#7 in reply to: ↑ 6 ; follow-up: @swissspidy
2 years ago

Something like add_theme_support( 'html5', [ 'script', 'css' ] );?

Yeah, something like that.

Could this be simplified to just add_theme_support( 'html5' ); to catch all?

That's how the command worked in the beginning, however it was changed to take an array of features that are supported.

For BC reasons add_theme_support( 'html5' ); is like calling add_theme_support( 'html5', array( 'comment-list', 'comment-form', 'search-form' ) );. I don't think we can change that. Plus, this would warrant its own ticket.

#8 in reply to: ↑ 7 @knutsp
2 years ago

Replying to swissspidy:

For BC reasons add_theme_support( 'html5' ); is like calling add_theme_support( 'html5', array( 'comment-list', 'comment-form', 'search-form' ) );. I don't think we can change that.

Why not?

Own ticket, of course.

@sasiddiqui
2 years ago

#9 @sasiddiqui
2 years ago

  • Keywords has-patch added; needs-patch removed

Please check this patch: https://core.trac.wordpress.org/attachment/ticket/42804/42804-update1.patch

You need to add the add_theme_support in functions.php to remove make the code work.

Something like:

add_theme_support( 'html5', array( 'comment-list', 'comment-form', 'search-form', 'script', 'style' ) );

or

add_theme_support( 'html5', array( 'script', 'style' ) );

#10 @swissspidy
2 years ago

#42977 was marked as a duplicate.

#11 @ocean90
2 years ago

#43079 was marked as a duplicate.

#12 @reeslo
23 months ago

This patch was not added to 4.9.2 ?

#13 @swissspidy
23 months ago

  • Milestone changed from Awaiting Review to 5.0

@reeslo No, this was not added in 4.9.2 as you can see from the history above. The ticket wasn't added to the 4.9.x milestone at any point. However, I think we can add this to 5.0.

#14 @swissspidy
23 months ago

#43111 was marked as a duplicate.

#15 @sasiddiqui
23 months ago

@swissspidy Can i own this ticket?

#16 @ocean90
17 months ago

#44570 was marked as a duplicate.

#17 follow-up: @pento
14 months ago

  • Milestone changed from 5.0 to 5.1

#18 in reply to: ↑ 17 ; follow-up: @gabrieldiggs
12 months ago

Replying to pento:
Has this been fixed yet? I'm getting the warnings in W3C validator.

EDIT: Added the following function to my theme functions.php and the W3C validation errors are gone. Is this the right way to fix the issue?

//Remove JS and CSS types

add_action( 'template_redirect', function(){
    ob_start( function( $buffer ){
        $buffer = str_replace( array( 'type="text/javascript"', "type='text/javascript'", 'type="text/css"', "type='text/css'" ), '', $buffer );

        return $buffer;
    });
});
Last edited 12 months ago by gabrieldiggs (previous) (diff)

#19 @pento
11 months ago

  • Milestone changed from 5.1 to Future Release

This patch needs review and a decision.

#20 @sasiddiqui
10 months ago

Is there any update when it can be deployed in WordPress Core? I can understand that it doesn't have such testing yet but it would be great if any time or WordPress version can be mentioned.

#21 @waterworks2
10 months ago

Any chance this will be reviewed and approved soon? Would like to clean up those remaining validation errors and get rid of unnecessary code. Thanks.

#22 @sasiddiqui
10 months ago

@waterworks2 We all are waiting for this.. Hopefully, it will be reviewed and available soon.

#23 @swissspidy
9 months ago

#46447 was marked as a duplicate.

#24 @dackar
6 months ago

Any chance this will be reviewed and approved soon? Would like to clean up those remaining validation errors and get rid of unnecessary code. Thanks.

#25 in reply to: ↑ 18 @daymobrew
5 months ago

Replying to gabrieldiggs:

EDIT: Added the following function to my theme functions.php and the W3C validation errors are gone. Is this the right way to fix the issue?

//Remove JS and CSS types

add_action( 'template_redirect', function(){
    ob_start( function( $buffer ){
        $buffer = str_replace( array( 'type="text/javascript"', "type='text/javascript'", 'type="text/css"', "type='text/css'" ), '', $buffer );

        return $buffer;
    });
});

I did something similar: https://www.damiencarbery.com/2018/11/remove-type-from-script-and-style-markup/
I filtered 'script_loader_tag' and 'style_loader_tag' to work on enqueued scripts and styles and used ob_start() and ob_get_contents() on 'wp_head' to process inline scripts and styles.

#26 @SergeyBiryukov
5 months ago

#47643 was marked as a duplicate.

#27 @SergeyBiryukov
3 months ago

#48038 was marked as a duplicate.

#28 @SergeyBiryukov
3 months ago

  • Milestone changed from Future Release to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#29 follow-up: @gabrieldiggs
3 months ago

Hello how do I leave this ticket? I have notifications blocked but I keep getting emails with updates. How do I block those?

#30 in reply to: ↑ 29 @SergeyBiryukov
3 months ago

Replying to gabrieldiggs:

Hello how do I leave this ticket? I have notifications blocked but I keep getting emails with updates. How do I block those?

Sorry for the noise, it's a known problem that's being investigated in #meta3594.

#31 @SergeyBiryukov
3 months ago

  • Keywords needs-dev-note added

#32 @SergeyBiryukov
3 months ago

In 46164:

Script Loader: Introduce HTML5 support for scripts and styles.

When a theme declares HTML5 support for script and styles via add_theme_support( 'html5', array( 'script', 'style' ) ), the type="text/javascript" and type="text/css" attributes are omitted.

These attributes are unnecessary in HTML5 and cause warnings in the W3C Markup Validation Service.

Props sasiddiqui, swissspidy, knutsp, SergeyBiryukov.
See #42804.

#33 @SergeyBiryukov
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46165:

Bundled Themes: Declare HTML5 support for scripts and styles.

Fixes #42804.

#34 @azaozz
3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Getting Fatal error: Uncaught Error: Call to undefined function current_theme_supports() in wp-includes\class.wp-scripts.php:145 in load-scripts.php.

load-scripts.php and load-styles.php do not load all of WP. They use sctipt-loader only as a whitelist for scripts and styles.

We can add a stub or a function_exists() for current_theme_supports() but then this (minor) enhancement becomes somewhat redundant? Also it seems it has effect only when outputting individual scripts and styles in wp-admin, i.e. when SCRIPT_DEBUG is true.

Also, would have been nice to also remove the <![CDATA[ stuff :)

Last edited 3 months ago by azaozz (previous) (diff)

#35 @SergeyBiryukov
3 months ago

In 46170:

Script Loader: Only check current theme's HTML5 support for scripts and styles on front end.

Avoids a fatal error in the admin if SCRIPT_DEBUG is disabled.

Props azaozz.
See #42804.

#36 @SergeyBiryukov
3 months ago

In 46171:

Script Loader: Move the current_theme_supports() check above the wp_default_(scripts|styles) action, for consistency.

See #42804.

@azaozz
3 months ago

#37 @azaozz
3 months ago

In 42804.2.diff:

  • Add function_exists() when using is_admin() and current_theme_supports(). At some point WP_Dependencies was a separate "package". It's very unlikely but "something" may still be using it that way...
  • Remove <![CDATA[ when outputting HTML 5.0 script tags. Not sure if it triggers validation errors, but is super outdated.

@SergeyBiryukov could you have a look in case I'm missing something and commit plz :)

#38 @davidbaumwald
3 months ago

  • Keywords commit added

#39 @SergeyBiryukov
3 months ago

  • Type changed from enhancement to task (blessed)

#40 @SergeyBiryukov
3 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 46287:

Script Loader: Add function_exists() checks for is_admin() and current_theme_supports(), to accomodate for using WP_Dependencies as a standalone class.

Remove <![CDATA[ when outputting HTML5 script tags.

Props azaozz.
Fixes #42804.

#41 @desrosj
8 weeks ago

  • Keywords needs-dev-note removed

Mentioned in the Miscellaneous Developer Focused Changes dev note for 5.3: https://make.wordpress.org/core/2019/10/15/miscellaneous-developer-focused-changes-in-5-3/

Note: See TracTickets for help on using tickets.