Easy stuff to do here:

1) get rid of the type attribute from <script>, <style> and <link> tags.
2) <meta http-equiv="content-type" content="text/html; charset=UTF-8"> should be: <meta charset="utf-8" />

Resources:
http://dev.w3.org/html5/spec/Overview.html#attr-script-type
http://dev.w3.org/html5/spec/Overview.html#attr-style-type
http://dev.w3.org/html5/spec/Overview.html#attr-link-type

http://diveintohtml5.org/semantics.html#encoding

Files: 
CommentFileSizeAuthor
#35 1174756-14-convert_head_markup_to_html5_34.patch2.59 KBkarschsp
PASSED: [[SimpleTest]]: [MySQL] 33,801 pass(es).
[ View ]
#29 1174756-14-convert_head_markup_to_html5_29.patch3 KBkarschsp
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1174756-14-convert_head_markup_to_html5_29.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#20 1174756-14-convert_head_markup_to_html5.patch2.95 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 33,630 pass(es).
[ View ]
#18 1174756-18-convert_head_markup_to_html5.patch2.92 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#14 1174756-14-convert_head_markup_to_html5.patch2.95 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 33,570 pass(es).
[ View ]
#3 1174756-convert_head_markup_to_html5-3.patch2.19 KBrupl
FAILED: [[SimpleTest]]: [MySQL] 30,017 pass(es), 9 fail(s), and 0 exception(es).
[ View ]
#1 1174756-convert_head_markup_to_html5-1.patch1.29 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 29,892 pass(es), 10 fail(s), and 0 exception(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.29 KB
FAILED: [[SimpleTest]]: [MySQL] 29,892 pass(es), 10 fail(s), and 0 exception(es).
[ View ]

And the patch.

Status:Needs review» Needs work

The last submitted patch, 1174756-convert_head_markup_to_html5-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.19 KB
FAILED: [[SimpleTest]]: [MySQL] 30,017 pass(es), 9 fail(s), and 0 exception(es).
[ View ]

It seems one of the SimpleTests became inaccurate with the proposed changes. Updated test included.

Status:Needs review» Needs work

The last submitted patch, 1174756-convert_head_markup_to_html5-3.patch, failed testing.

Status:Needs work» Needs review

I don't get these fails locally, so let's see if this was a testbot issue.

Issue tags:-html5

Status:Needs review» Needs work
Issue tags:+html5

The last submitted patch, 1174756-convert_head_markup_to_html5-3.patch, failed testing.

Subscribing

Tagging. Would be nice to make some progress on this one by next week. :)

I may be able to help debug the test, subscribing for now and hopefully will have a chance later on today to debug.

Just wanted to confirm that this does fail on my local machine. Seeing if I can debug now.

+++ b/includes/common.incundefined
@@ -310,8 +310,7 @@ function _drupal_default_html_head() {
     '#type' => 'html_tag',
     '#tag' => 'meta',
     '#attributes' => array(
-      'http-equiv' => 'Content-Type',
-      'content' => 'text/html; charset=utf-8',
+      'charset' => 'utf-8',
     ),

This is the part that is causing the fail.

It's failing on Español.

I'm guessing that this is an issue because xpath can't figure out what the character encoding is and is getting mixed up on the special character. DrupalWebTestCase uses PHP's DOMDocument to handle the HTML of the page. It seems that at least some versions of PHP didn't have parsing rules for HTML5 and don't read the charset attribute as containing the character encoding (this is probably why it worked on your machine amateescu, you probably have a newer version of PHP installed).

If I'm on the right track, then there is some discussion of this issue at http://ie.php.net/manual/en/domdocument.loadhtml.php

Status:Needs work» Needs review
StatusFileSize
new2.95 KB
PASSED: [[SimpleTest]]: [MySQL] 33,570 pass(es).
[ View ]

This dirty hack should do the trick...

+++ b/modules/simpletest/drupal_web_test_case.php
@@ -1677,7 +1677,7 @@ class DrupalWebTestCase extends DrupalTestCase {
       // DOM can load HTML soup. But, HTML soup can throw warnings, suppress
       // them.
       $htmlDom = new DOMDocument();
-      @$htmlDom->loadHTML($this->drupalGetContent());
+      @$htmlDom->loadHTML('<?xml encoding="UTF-8">' . $this->drupalGetContent());

Status:Needs review» Needs work

+++ b/includes/common.incundefined
@@ -4138,9 +4133,7 @@ function drupal_get_js($scope = 'header', $javascript = NULL, $skip_alter = FALS
-    '#attributes' => array(
-      'type' => 'text/javascript',
-    ),

Nit-picky, but cant we just get rid of the '#attributes here'

8 days to next Drupal core point release.

I have no strong opinion on the issue. Most user agents already default to "text/javascript" if it isn't explicitly stated, but there could be some that still don't. In light of this, I would lean towards leaving it in, since it doesn't harm anything, but no real preference.

linclark - sorry, my code snippit got cut off a bit so I think you misunderstood my meaning...

+++ b/includes/common.incundefined
@@ -4138,9 +4133,7 @@ function drupal_get_js($scope = 'header', $javascript = NULL, $skip_alter = FALS
     '#value' => '',
-    '#attributes' => array(
-      'type' => 'text/javascript',
-    ),
+    '#attributes' => array(),

I was not suggesting keeping the "type" attribute, I just wasnt sure if we needed to set "#attributes" equal to an empty array. I'm simply suggesting that we remove the attributes as the patch already does without adding the new line seen here: '#attributes' => array();

7 days to next Drupal core point release.

Status:Needs work» Needs review
StatusFileSize
new2.92 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Oooh, so we already don't do what I was leaning towards....

Yeah, I would think you are correct, no reason to set empty attributes. Rerolling...

Status:Needs review» Needs work

The last submitted patch, 1174756-18-convert_head_markup_to_html5.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.95 KB
PASSED: [[SimpleTest]]: [MySQL] 33,630 pass(es).
[ View ]

Ok, in checking this manually, we totally need attributes there... otherwise theme_html_tag goes batty.

Reposting the previous patch.

Still needs to be reviewed and tested. Updating to current sprint.

+++ b/includes/common.incundefined
@@ -3223,16 +3222,12 @@ function drupal_pre_render_styles($elements) {
   $style_element_defaults = array(
     '#type' => 'html_tag',
     '#tag' => 'style',
-    '#attributes' => array(
-      'type' => 'text/css',
-    ),
   );

No problems on my local environment! One minor thing I noticed was that <style> doesn't retain the empty #attributes array in the same manner as <script>.

Assigned:amateescu» Unassigned
Status:Needs review» Needs work
Issue tags:-HTML5 Sprint: July 2011 - 2, -HTML5 Sprint: August 2011 - 2

One minor thing I noticed was that doesn't retain the empty #attributes array in the same manner as .

Hmm. If we have '#attributes' => array() for the script, then why would we omit it for style tags? Maybe I'm missing something. Setting to needs work for that, until I hear back.

Also, un-assigning this from amateescu (feel free to reassign yourself if you're going to work on it) and cleaning up tags.

Status:Needs work» Needs review

@Jacine is because the style tags has the rel attributes. The script tag technically should be empty but since the html_tag element requires an attributes declaration the empty array() was added.

Switching to needs-review as per this is what was blocking it before.

I didn't actually test this patch.

Status:Needs review» Needs work
Issue tags:+Novice

Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

Tagging as novice for the task of rerolling the Drupal 8.x patch.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

Issue tags:+sprint

Tagging this for the next sprint.

Status:Needs work» Needs review
StatusFileSize
new3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1174756-14-convert_head_markup_to_html5_29.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Here's a re-roll for the /core change.

Status:Needs review» Needs work
Issue tags:-Novice, -html5, -sprint

The last submitted patch, 1174756-14-convert_head_markup_to_html5_29.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+Novice, +html5, +sprint

The last submitted patch, 1174756-14-convert_head_markup_to_html5_29.patch, failed testing.

Can you verify whether this patch applies locally? Testbot is unable to apply it.

Status:Needs work» Needs review

It didn't. Here's a re-roll that should apply cleanly.

Status:Needs review» Needs work
StatusFileSize
new2.59 KB
PASSED: [[SimpleTest]]: [MySQL] 33,801 pass(es).
[ View ]

Yeah, and the patch. TGIF!

Status:Needs work» Needs review

Setting to needs review for testbot.

testbot liked it, is this ready to test?

Edit: Editing comments is cheating. ;) I'd say test away!

Manually tested,

Everything applies, everything looks good when viewing source code.

This in addition to the unit tests should be enough to mark this a RTBC.

Status:Needs review» Reviewed & tested by the community

Perhaps I'm jumping the gun with promoting this issue to RTBC but it looks pretty simple and pretty clean. And I was able to apply the patch and test it manually.

Whoa, what.

-      'http-equiv' => 'Content-Type',
-      'content' => 'text/html; charset=utf-8',

why? (http://diveintohtml5.org/semantics.html#encoding is not available)

Because of the line that follows those deletions :)

+++ b/core/includes/common.inc
@@ -299,8 +299,7 @@ function _drupal_default_html_head() {
-      'http-equiv' => 'Content-Type',
-      'content' => 'text/html; charset=utf-8',
+      'charset' => 'utf-8',

http://www.w3.org/TR/html5-diff/#syntax

@chx that page has been relocated to this URL: http://diveintohtml5.info/semantics.html#encoding

Several paragraphs in, it provides as an example of a valid HTML5 charset meta tag, which the current patch produces:

<meta charset="utf-8" />

Good. Then all concerns have been addressed?

Status:Reviewed & tested by the community» Needs work

Absolutely not. The link @rupl provided says "Both of these techniques still work in HTML5. " meaning the HTTP header and the http-equiv meta we used so far. Did anyone test this with IE? This is not some "meh, it will look garbled, but who cares" issue. If a page has only ASCII data and so no way for IE to detect a charset and the user provides some messed up Windows charset are we going to put broken data into the database? What's the point of the change if we know the old one will still work and basically no idea (at least not in the issue) whether it can lead to serious problems under IE?

Issue tags:+Needs manual testing

Issue tags:-Needs manual testing

redacting following discussion in irc.

Status:Needs work» Reviewed & tested by the community

It's completely superflous anyways, turns out http://api.drupal.org/api/function/drupal_deliver_html_page/7 sends the HTTP header that the meta provides an equivalent of. Yes, we could go on and on about broken HTTP proxies and whatnot but let's not and go ahead with this. It's just a safety belt on top of a harness anyways.

@chx, let me know which manual tests you'd like to have done for this patch and I'll report back what I find.

I have the full gambit of browsers that I can send them through. I tested this patch with all of them and didn't see anything wrong at first glance.

@cosmicdreams No testing is needed (the tag was added and then removed). The issue was marked back to RTBC after a discussion in IRC. It has been and still is ready, so we're just waiting for a commit.

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.