WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#18788 assigned enhancement

Remove redundant type attributes from script and style tags

Reported by: solarissmoke Owned by: wonderboymusic
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3
Component: Script Loader Keywords: 3.4-early has-patch
Focuses: administration Cc:

Description

Now that the admin is using the HTML5 doctype everywhere, the type="text/javascript" and type="text/css" attributes on script and style tags are unnecessary (if they weren't anyway), and I think they can be safely removed to trim a few hundred bytes from core. Should be a simple search and replace exercise.

Attachments (2)

no-cdata.diff (10.2 KB) - added by tw2113 3 years ago.
removed script CDATA tags. Left script types on though.
no-cdata1.diff (12.2 KB) - added by tw2113 3 years ago.
forgot wp-includes

Download all attachments as: .zip

Change History (25)

comment:1 @azaozz4 years ago

  • Keywords 3.4 added
  • Milestone changed from Awaiting Review to Future Release

We have to confirm that first for older mobile browsers. They still might be needed for some. Also the //<![CDATA[ bits inside of <script> are not needed.

comment:2 @tw21133 years ago

  • Cc michael.d.beckwith@… added

@tw21133 years ago

removed script CDATA tags. Left script types on though.

@tw21133 years ago

forgot wp-includes

comment:3 @tw21133 years ago

  • Keywords has-patch added

I did my best to stick to just <script></script> blocks. I didn't edit any of them in RSS/XML based feed files.

comment:4 @retlehs2 years ago

  • Cc retlehs added

comment:5 @Volker_E.2 years ago

  • Cc Volker_E. added

comment:6 @SergeyBiryukov2 years ago

  • Keywords 3.4 removed

HTML comment tags in wp-admin/includes/file.php can also be removed:
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-admin/includes/file.php#L969

comment:7 @SergeyBiryukov2 years ago

  • Keywords 3.4-early added

Fixing the keyword for reference.

comment:9 @nacin16 months ago

  • Component changed from Administration to Script Loader
  • Focuses administration added

comment:10 @wonderboymusic5 months ago

  • Milestone changed from Future Release to 4.2

comment:11 @wonderboymusic5 months ago

In 31034:

Inline <script>s that are only printed in the admin for pages that are served with the HTML5 doctype absolutely do not need CDATA comments.

Props tw2113 for the initial patch.
See #18788.

comment:12 follow-up: @wonderboymusic5 months ago

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

type is optional in HTML5, not required. I don't think it hurts to keep it.

comment:13 in reply to: ↑ 12 ; follow-up: @retlehs5 months ago

Replying to wonderboymusic:

type is optional in HTML5, not required. I don't think it hurts to keep it.

Since it's optional, why keep it?

comment:14 in reply to: ↑ 13 ; follow-up: @AramZS4 months ago

Replying to retlehs:

Replying to wonderboymusic:

type is optional in HTML5, not required. I don't think it hurts to keep it.

Since it's optional, why keep it?

Always better to be more specific than less, right?

comment:15 in reply to: ↑ 14 @solarissmoke4 months ago

Replying to AramZS:

Always better to be more specific than less, right?

The HTML5 specification is explicitly clear about what a missing type attribute means, so specificity isn't an issue. Removing it means less bloat and easier code to read.

comment:16 follow-up: @retlehs4 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening because type still isn't removed.

comment:17 @slackbot2 months ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:18 @DrewAPicture2 months ago

  • Keywords 4.2-beta added

@wonderboymusic: Any idea what's left here? Letting this ride into Beta 1, see [31034].

comment:19 in reply to: ↑ 16 ; follow-up: @helen2 months ago

Replying to retlehs:

Reopening because type still isn't removed.

Curious if anybody tried checking into that on older browsers, per the first comment from azaozz.

comment:20 in reply to: ↑ 19 @retlehs2 months ago

Replying to helen:

Curious if anybody tried checking into that on older browsers, per the first comment from azaozz.

Is there a list of supported browsers that need to be tested? It's fine in IE8. It's probably fine in IE7, but I don't have a VM with it up and running yet.

(Not sure if any of this even matters considering a HTML5 doctype is used)

Last edited 2 months ago by retlehs (previous) (diff)

comment:21 @DrewAPicture2 months ago

  • Owner set to wonderboymusic
  • Status changed from reopened to assigned

comment:22 @slackbot2 months ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:23 @DrewAPicture2 months ago

  • Keywords 4.2-beta removed
  • Milestone changed from 4.2 to Future Release

Per consensus in the above-linked Slack discussion, punting this to a future release.

Note: See TracTickets for help on using tickets.