Related to #1140356: Add async, onload property to script tags.

These lines from common.inc...

<?php // Element properties that do not depend on item type.
 $element = $element_defaults;
 if (!empty($item['defer'])) {
   $element['#attributes']['defer'] = 'defer';
 }
?>

...Are in direct conflict with this draft HTML 5 spec:

The defer and async attributes must not be specified if the src attribute is not present.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

iamEAP’s picture

Status: Active » Needs review
Issue tags: +html5
FileSize
1.37 KB

Preliminary patch that takes care of this.

Also tagged html5.

arnested’s picture

Yes, 'defer' should only be added to file and external and the patch works fine.

nod_’s picture

how about unset-ing #attributes['defer'] in inline scripts? that would avoid some duplication.

iamEAP’s picture

How about if we move it after the switch/case block? We can check for an src attribute and line up directly with the spec definition.

iamEAP’s picture

I might say this needs backporting too. Tagging.

arnested’s picture

Status: Needs review » Reviewed & tested by the community

Yes, much better approach in #4.

Reviewed and tested. Works like a charm.

Dries’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x. Moving to 7.x.

iamEAP’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
iamEAP’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.95 KB

Going ahead and reducing redundancy in D7.

iamEAP’s picture

Same, but doesn't wipe unrelated comments.

ericduran’s picture

For D7, I think is best to just mark this issue as closed and instead fix defer and async in one shot over at #1140356: Add async, onload property to script tags. The other patch fixes this issue and also adds test.

tim.plunkett’s picture

Issue tags: +Needs backport to D7

Fixing tag.

ericduran’s picture

I think this should just be marked as dupe but I'll leave it to someone else.

#1140356: Add async, onload property to script tags already got in to D8, which is a better fixed (on top of this D8 patch) for this problem and that one is also marked for backport. But 1st #1587536: JavaScript aggregation should account for "async" and "defer" attributes needs to be fixed.

So I don't think this issue is needed.

iamEAP’s picture

Yeah, there are some pros and cons to that.

On the one hand, the two (three) issues are very tightly knit and a backport for #1140356: Add async, onload property to script tags would depend on this.

On the other hand, this is still a separate bug with an easy fix. It's likely the aforementioned issue (along with #1587536: JavaScript aggregation should account for "async" and "defer" attributes will probably stay active for at least another month. No use in having this similar but distinct bug remain a bug while a new feature is added and backported.

  • Dries committed 8780ba2 on 8.3.x
    - Patch #1503804 by iamEAP: defer attribute should not be added to...

  • Dries committed 8780ba2 on 8.3.x
    - Patch #1503804 by iamEAP: defer attribute should not be added to...

  • Dries committed 8780ba2 on 8.4.x
    - Patch #1503804 by iamEAP: defer attribute should not be added to...

  • Dries committed 8780ba2 on 8.4.x
    - Patch #1503804 by iamEAP: defer attribute should not be added to...

  • Dries committed 8780ba2 on 9.1.x
    - Patch #1503804 by iamEAP: defer attribute should not be added to...