Add <code> element inside applicable <pre> elements

[Zirro]

Resolves #3753 and #3764.

This is not ready to be merged yet as it'll also require a few changes to the build system and CSS to target <code> elements rather than <pre> elements. I'd also like to go over it again to make sure it's correct in all locations.

I added an explicit html class rather than having it be implicit as was the case before, since there are some non-HTML/CSS/JS/IDL <pre>s as well. Figured it might come in handy some day.

@sideshowbarker, would these changes work well for you once the syntax highlighting has been adjusted to account for the <code> elements?

---

/browsers.html ( diff )
/browsing-the-web.html ( diff )
/canvas.html ( diff )
/common-dom-interfaces.html ( diff )
/common-microsyntaxes.html ( diff )
/comms.html ( diff )
/custom-elements.html ( diff )
/dnd.html ( diff )
/dom.html ( diff )
/edits.html ( diff )
/embedded-content-other.html ( diff )
/embedded-content.html ( diff )
/form-control-infrastructure.html ( diff )
/form-elements.html ( diff )
/forms.html ( diff )
/grouping-content.html ( diff )
/history.html ( diff )
/iframe-embed-object.html ( diff )
/image-maps.html ( diff )
/imagebitmap-and-animations.html ( diff )
/images.html ( diff )
/input.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/introduction.html ( diff )
/links.html ( diff )
/media.html ( diff )
/microdata.html ( diff )
/obsolete.html ( diff )
/offline.html ( diff )
/parsing.html ( diff )
/rendering.html ( diff )
/scripting.html ( diff )
/sections.html ( diff )
/semantics-other.html ( diff )
/semantics.html ( diff )
/server-sent-events.html ( diff )
/structured-data.html ( diff )
/syntax.html ( diff )
/system-state.html ( diff )
/tables.html ( diff )
/text-level-semantics.html ( diff )
/timers-and-user-prompts.html ( diff )
/web-messaging.html ( diff )
/web-sockets.html ( diff )
/webappapis.html ( diff )
/webstorage.html ( diff )
/window-object.html ( diff )
/workers.html ( diff )

[sideshowbarker]

@sideshowbarker, would these changes work well for you once the syntax highlighting has been adjusted to account for the <code> elements?

In wattsi and the html-build sources, the code related to syntax highlighting is agnostic toward whether or not there’s a code element there. It all just passes any contents of the pre elements through to the highlighter script.

So there’s nothing special about those code elements as far as our build goes.

In other words, there are no build adjustments to be made on our side for this.

But whether the external highlighter script that we call needs to be adjusted to do something different because of those elements, I dunno.

[domenic]

Wow, thanks @Zirro!

@sideshowbarker, it seems like Zirro moved the class="lang" from the <pre> to the <code>, following the spec's suggestion. Since classes aren't semantic anyway, I'm not sure that move is really necessary, but it might be worthwhile to align with the spec if it isn't too much trouble for you...

In that case, we'd need to adjust wattsi, right? Hmm.

[tabatkins]

Nope, my highlighter is fine, it doesn't care one bit whether there's a code or not (or any other markup).

You'll just need to change wattsi to look for the class on the code to know what language to highlight as.

[sideshowbarker]

it seems like Zirro moved the class="lang" from the <pre> to the <code>

That seems unnecessary and I strongly prefer this not be landed that way

following the spec's suggestion

I suggest we not follow that suggestion :) And further I think we should consider also changing the spec to not suggest that, because it’s not objectively a better way to do the markup by any measure

Since classes aren't semantic anyway, I'm not sure that move is really necessary

Right

but it might be worthwhile to align with the spec if it isn't too much trouble for you… In that case, we'd need to adjust wattsi, right?

Yeah, we’d need to adjust the wattsi code, and it’d be significantly more trouble to adjust it to look for a code first child of each pre and get the classname from that.

The cost of doing that would still be worthwhile if there were a compelling benefit. But it doesn’t seem like there is any benefit compelling enough to merit the cost

[domenic]

Makes sense. @Zirro, any objections to moving the class to the pre?

I guess one reason the spec might suggest this is that you could then use the same highlighter code for inline code samples, not just big pre blocks.

[Zirro]

@domenic That's fine with me. Done, and rebased on latest master.

[sideshowbarker]

I don’t support adding code children to all the pre elements. It’s not necessary and in fact we have some evidence that it may actually measurably more harmful than not using code children in pre.

See @tabatkins comment at #2751 (comment), where he indicates that the CSS specs are very intentionally not using code children in their pre blocks because:

Probably for the same reason we just use pre in the CSS specs - <pre><code> makes the magical-newline-omitting of pre work even worse than pre by itself.

So I think the change we instead should be making here is to update the suggestions and examples in the spec to align with what most authors actually seem to be doing (and what the CSS specs are doing) — that is, to not suggest using code children as the first child of every single pre element that has an example of JavaScript or HTML or CSS or whatever in it.

I’ll create a PR with a patch for that.

[sideshowbarker]

I added an explicit html class rather than having it be implicit as was the case before, since there are some non-HTML/CSS/JS/IDL <pre>s as well. Figured it might come in handy some day.

Yeah — that’s a great change. I really think we need that. Lacking that at this point, I’ve needed to make the syntax-highlighter code assume that any pre element without an explicit class should get highlighted as HTML. But as you note, in the spec there some pre elements without class attributes which aren’t HTML but instead just an example of a poem or something. So as far as the syntax highlighter goes, we really need an explicit class on the HTML pre elements so that we can highlight those as intended — while skipping highlighting for the ones that aren’t HTML or CSS or JavaScript.

[domenic]

I guess we should wait to merge this until the discussion in #3772 gets a bit further. In the meantime, let me put the commit message I drafted here for safe keeping:

Editorial: mark up code blocks with their language and <code>

Fixes #3753 (for JS) and also marks up the HTML examples with their own
class. Fixes #3764.

This will help with ongoing work to syntax-highlight all code in the
spec; see https://github.com/whatwg/html/issues/3753.

[sideshowbarker]

Upon further consideration I’m dropping my objection to this change, and I have a patch ready to land that aligns with this PR, by adding support to Wattsi for sending code contents thru the highlighter pipeline in the case where the code element has a class value with idl, js, css, or html.

So @Zirro are you still able to roll back this branch to the original state of the patch at the time when you initially raised the PR? If so, then I think that would be the better way to land this. I mean specifically, with the class attribute on the code element rather than the pre element.

[Zirro]

Okay, I've moved the classes back over to the code elements. It looks like some of the shared CSS will also need adjustment before this is merged, though.

[sideshowbarker]

There’s an pre element at line 734 of the source on this branch, with id=intro-early-example. It contains an HTML markup example but <code class="html">…</code> hasn’t been added around its contents. Is that intentional?

[Zirro]

Thanks for catching that. It looks like I missed pre elements with attributes other than class or with other classes than example. This seems to affect 31 elements - the one you found, in addition to pre elements with the bad class (edit: ...and a few others). I'll fix this over the weekend.

[sideshowbarker]

Thanks for catching that. It looks like I missed pre elements with attributes other than class or with other classes than example. This seems to affect 31 elements - the one you found, in addition to pre elements with the bad class. I'll fix this over the weekend.

Cool. I think it may affect a few more than just those. Search for <pre>EXAMPLE in the source. There are 30 of those — mostly HTML and JS examples, with one CSS example.

And search for <pre lang="[^"]+">&lt; in the source and you should find 6 more cases of HTML examples.

[sideshowbarker]

I'm ready to merge this, and whatwg/html-build#162, once whatwg/whatwg.org#215 and whatwg/whatwg.org#214 get approvals. It'd also be good to get @sideshowbarker's final signoff.

Reviewed here and approved — and also approved the whatwg/whatwg.org PRs.

So I think we’re finally ready to merge all this.

[sideshowbarker]

So I meant to say in my previous comment that the Travis CI check is now green because I figured out and fixed the problem.

The cause was 100% my flub… I introduced a regression in wattsi’s behavior in whatwg/wattsi@b7d18be#diff-1922eb90770bdd34ac96cc9db339faebL1927.

It almost certainly isn’t caused by the HTML that wattsi generates, because as far as I can see, wattsi isn’t actually capable of generating HTML output without the doctype — it’s hardcoded

I was wrong about that. That had it hardcoded to call a function to conditionally write the doctype — but the assumption I wrote that with didn’t hold up for the case when writing the index-html files with syntax highlighting not enabled.

So in whatwg/wattsi@f3e6a62 I fixed (unregressed) it to (once again) unconditionally emit the doctype when writing HTML files.