WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 months ago

#18788 closed enhancement (fixed)

Remove redundant type attributes from script and style tags

Reported by: solarissmoke Owned by: wonderboymusic
Milestone: 4.2 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 4 years ago.
removed script CDATA tags. Left script types on though.
no-cdata1.diff (12.2 KB) - added by tw2113 4 years ago.
forgot wp-includes

Download all attachments as: .zip

Change History (26)

#1 @azaozz
4 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.

#2 @tw2113
4 years ago

  • Cc michael.d.beckwith@… added

@tw2113
4 years ago

removed script CDATA tags. Left script types on though.

@tw2113
4 years ago

forgot wp-includes

#3 @tw2113
4 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.

#4 @retlehs
3 years ago

  • Cc retlehs added

#5 @Volker_E.
3 years ago

  • Cc Volker_E. added

#6 @SergeyBiryukov
3 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

#7 @SergeyBiryukov
3 years ago

  • Keywords 3.4-early added

Fixing the keyword for reference.

#9 @nacin
2 years ago

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

#10 @wonderboymusic
13 months ago

  • Milestone changed from Future Release to 4.2

#11 @wonderboymusic
13 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.

#12 follow-up: @wonderboymusic
13 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.

#13 in reply to: ↑ 12 ; follow-up: @retlehs
13 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?

#14 in reply to: ↑ 13 ; follow-up: @AramZS
13 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?

#15 in reply to: ↑ 14 @solarissmoke
13 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.

#16 follow-up: @retlehs
12 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening because type still isn't removed.

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


11 months ago

#18 @DrewAPicture
11 months ago

  • Keywords 4.2-beta added

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

#19 in reply to: ↑ 16 ; follow-up: @helen
11 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.

#20 in reply to: ↑ 19 @retlehs
11 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 11 months ago by retlehs (previous) (diff)

#21 @DrewAPicture
11 months ago

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

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


11 months ago

#23 @DrewAPicture
11 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.

#24 @wonderboymusic
4 months ago

  • Milestone changed from Future Release to 4.2
  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.