-
Notifications
You must be signed in to change notification settings - Fork 547
Conversation
* Indentation * Unicode Fixes #1182
* Indentation * Unicode Fixes #1182
* Fix alt text * Remove bitly * Fix broken links * Update examples to match the example given Fixes #1182
* Simplify Examples * Fill spelling and grammar * Beautify Fixes #1182
* Simplify examples Fixes #1182
* Invisible Times (!) made visible. * MathML example to be displayed Re #1182
* Clarify spellcheck Re #1182
* Simplified several examples * Added highlights where possible Re #1182
* Simplified examples * Reduced verbiage * Corrected errors Re #1182
* Simplified examples Re #1182
* Simplified examples (dramatically in some cases) * Fixed errors * Updated to modern style Re #1182
* Simplified * Fixed broken markup Re #1182
* Better example text (out of copyright) * Improved formatting * `sh` isn't a language code(!) Re #1182
* Improved abbr * Better quotes re #1182
Wondering if we should merge this and then continue review filing issues for particular things, or get the whole review done first. I don't mind either way... |
sections/semantics.include
Outdated
<p> <label for=c> <input id=a> </label> <span id=b> <input id=c> </span> </p> | ||
</pre> | ||
<xmp highlight="html"> | ||
<p> <label for=c> <input id=a> </label> <span id=b> <input id=c> </span> </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add quotes around attribute values?
:: <pre highlight="html"> | ||
<source src='video.mp4' type='video/mp4; codecs="avc1.42E01E, mp4a.40.2"'> | ||
</pre> | ||
:: <xmp highlight="html"><source src='video.mp4' type='video/mp4; codecs="avc1.42E01E, mp4a.40.2"'></xmp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we inverse single and double quotes in the source tag? (I'm good either way...). This goes for this line as well as the next 12 examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, whatever we do we need to do for all examples with the codecs attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
</pre> | ||
<xmp highlight="html"> | ||
<ul> | ||
<li><button><img src="b.png" alt="Bold"></button></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no closing tag for img. We either add it or modify the style guide. (My preference was no closing tag to img but I think I was the only one...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer it to be <img .... />
- I've not made the changes yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer ... />
too
</pre> | ||
<xmp highlight="html"> | ||
<a href="https://w3.org"> | ||
<img src="images/w3c_home.png" width="72" height="48" alt="W3C web site"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above (closing tag to img) (there is a number of examples on this page where this comment would apply. Some already have a closing tag.
</pre> | ||
<xmp highlight="html"> | ||
<h2>From today's featured article</h2> | ||
<img src="/uploads/100-marie-lloyd.jpg" alt="A singer on the stage" width="100" height="150"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as previously... question about img closing tag
<caption>Specification values: <b>Steel</b>, <b>Castings</b>, Ann. A.S.T.M. A27-16, Class B;* P max. 0.06; S max. 0.05.</caption> | ||
<thead> | ||
<tr> | ||
<th rowspan=2>Grade.</th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure, but wondering whether 2 should be in quotes or is it as is intentionally? (also next example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed it. My grep was looking for words after =. Didn't think about numbers! D'oh!
<xmp highlight="html"> | ||
<p>For instance, this fantastic sentence has bullets relating to</p> | ||
<ul> | ||
<li>wizards,<li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should <li>
at end of lines be </li>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot!
<xmp highlight="html"> | ||
<div>For instance, this fantastic sentence has bullets relating to | ||
<ul> | ||
<li>wizards,<li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above (eol <li>
-> </li>
)
<div> | ||
<p>For instance, this fantastic sentence has bullets relating to</p> | ||
<ul> | ||
<li>wizards,<li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above (eol <li>
-> </li>
)
sections/browsers.include
Outdated
@@ -5103,7 +5103,7 @@ | |||
</head> | |||
|
|||
<body onload="updateIndicator()" ononline="updateIndicator()" onoffline="updateIndicator()"> | |||
<p>The network is: <span>(state unknown)</span> | |||
<p>The network is: <span>(state unknown)</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there still be a </span>
before </p>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple more things...
sections/semantics-sections.include
Outdated
</style> | ||
<xmp highlight="html"> | ||
<style> | ||
section { border: "double medium"; margin: "2em"; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some quotes that should not be in CSS
sections/semantics-sections.include
Outdated
<xmp highlight="html"> | ||
<nav> | ||
<h2>Navigation</h2> | ||
<p>You are on my home page. To the north lies <a href="/blog">my blog</a>, from whence the sounds of battle can be heard. To the east you can see a large mountain, upon which many <a href="/school">school papers</a> are littered. Far up this mountain you can spy a little figure who appears to be me, desperately scribblinga <a href="/school/thesis">thesis</a>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"from whence" is tautological, redundantly repeating the same concept twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And should "scribblinga" be "scribbling on a"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's good enough for Shakespeare, it's good enough for the spec :-) https://english.stackexchange.com/questions/10906/is-from-whence-correct-or-should-it-be-whence
I'll fix the scribbling.
sections/infrastructure.include
Outdated
|
||
You can also mix styles; the following is still equivalent: | ||
You can also mix styles; the following is also equivalent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure this is better. This now says “also” twice.
|
||
<pre class="bad" highlight="html"><a href="?art&copy">Art and Copy</a></pre> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does <xmp>
know to escape &
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :-)
|
||
CSS: | ||
<xmp highlight="css"> | ||
details>summary { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May the selector spacing be preserved here? e.g. details > summary
. That would be more readable and consistent with the choice to preserve spacing between properties and values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Note to self - https://www.w3.org/TR/CSS2/selector.html#child-selectors
</p> | ||
</pre> | ||
<xmp highlight="html"> | ||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HTML5 Boilerplate project already went through the hot drama of this and ended up going with lowercase conformity (<!doctype html>
), which has then trickled down into other projects. However, if we’re going to kick the dust back up and uppercase part or all of this, I’d hope everyone plays nice and aligns on one defacto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first pass was to make it consistent throughout the examples. Discussion on what it should be is taking place in #1193
I'm happy either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
started bottom up, pointed out some unescaped strings and suggested rephrasing.
@@ -6647,7 +6667,7 @@ | |||
following markup: | |||
|
|||
<pre highlight="html"> | |||
<table><b><tr><td>aaa</td></tr>bbb</table>ccc | |||
<table><mark><b></mark><tr><td>aaa</td></tr>bbb</table>ccc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing unescape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean. The <mark>
element has to be unescaped in order to highlight the text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i meant the rest of the tags. so, if i got this right, if a block of code has a <mark>
, none of it is transformed into <xmp highlight="html">
and unescaped ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
presumably your comment was "has a <mark>
element ...
In which case the answer is yes. We keep using <pre>
if we want to use actual markup on the example code...
<xmp highlight="html" class="bad"><a href="a">a<table><a href="b">b</table>x</xmp> | ||
The first <{a}> element would be closed upon seeing the second one, and the "x" character | ||
would be inside a link to "b", not to "a". This is despite the fact that the outer | ||
<{a}> element is not in table scope (meaning that a regular </a> end tag at the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing unescape.
@@ -362,74 +362,74 @@ | |||
<div class="example"> | |||
For example, in the following case it's ok to remove the "<code><html></code>" tag: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing unescape in line 363 and in line 356.
|
||
With the tag removed, the document actually turns into the same as this: | ||
|
||
<pre highlight="html"> | ||
<!DOCTYPE HTML> | ||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these next lines are missing unescape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional. There is no way to add a <mark>
into an <xmp>
element and have it show up as highlighted text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is answers my question above :)
<p>Welcome to this example.</p> | ||
</body> | ||
</html> | ||
</xmp> | ||
|
||
(The <{body}> and <{html}> element end tags could be omitted without trouble; any spaces after | ||
those get parsed into the <{body}> element anyway.) | ||
|
||
Usually, however, white space isn't an issue. If we first remove the white space we don't care |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about rephrasing this as:
"If the white space is not important, it can be removed:"
If so, the next lines could also be changed.
</pre> | ||
<xmp highlight="html"> | ||
<!DOCTYPE html><title>Hello</title><p>Welcome to this example.</p> | ||
</xmp> | ||
|
||
At that point, we can also add some white space back: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"At that point, some white space can be added back:"
@@ -528,7 +528,7 @@ | |||
We can thus simplify the earlier example further: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Thus, the earlier example can be simplified further:"
@@ -912,13 +918,13 @@ | |||
In the following example, the tree construction stage will be called upon to handle a "`p`" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some references to tags use <{p}>, and in this case to
"`p`"
... | ||
</pre> | ||
</xmp> | ||
</div> | ||
|
||
To handle these cases, parsers have a <dfn>script nesting level</dfn>, which must be initially set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
```
<p class="note">The DOM will not let {{Document}} nodes contain {{Text}} nodes, so they
are ignored.</p>
```
sections/syntax.include
Outdated
<td> | ||
<xmp highlight="html"> | ||
A<script> | ||
var text = document.createTextNode('B'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra whitespace.
* Re #1199 (comment) * Re #1182
* Re #1199 (comment) * Re #1182
* re #1199 (comment) * Re #1182
* Re #1199 (comment) * Re #1182
* Fix #1199 (comment) * Re #1182
sections/browsers.include
Outdated
</pre> | ||
<xmp highlight="html"> | ||
<!DOCTYPE html> | ||
<title>Line</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we wrap this in a head
? Other examples in this file do so, and there's a body
in this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. Fixing it now.
* Re #1199 (review) * Re #1182
🎉 Thanks for getting this in! Is it live? I’m seeing odd indentation in the Drag and Drop Introduction Example. |
@jonathantneal there may be some instances where we need to do some additional clean up. I just did some other minor edits to that file & example, so once that is published, we'll see if the indenting is still an issue. |
@@ -832,12 +832,11 @@ | |||
<div class="example"> | |||
The following examples illustrate code that should be avoided: | |||
<pre highlight="html"> | |||
<!-- DO NOT DO THIS --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 stray < s - lines 835, 837, 839
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, these are in a pre
element, with live <mark>
elements so specific bits get highlighted. So the code for display needs to be escaped as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chaals is right. They need to stay escaped.
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<title>This is an example for the <base> element</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray < and >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't stray. They're needed to display the <base>
as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually you don't within a title
element. It uses the RCDATA parsing path so the text goes into the DOM
I am not sure if I would recommend writing HTML this way, though. Question for the Style guide #1193 I suspect
Please note This Pull Request has been merged. Changes should be suggested as new issues, and made as new Pull Requests. |
Fixes #1182
Please feedback any mistakes.
Ridiculously large, sorry!