New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'amp' in html tag not valid in HTML5? #472

Closed
IanCal opened this Issue Oct 7, 2015 · 17 comments

Comments

Projects
None yet
7 participants
@IanCal
Copy link

IanCal commented Oct 7, 2015

Had a look through the open and closed issues but couldn't see this, apologies if it's a duplicate.

The specifications require:

  • start with the doctype <!doctype html>. πŸ”—
  • contain a top-level <html ⚑> tag or <html amp>

However this:

<!doctype html>
<html amp>
<head><title>title</title></head>
</html>

Does not pass the validator here: https://validator.w3.org/nu/#textarea

It fails with the error:

Error: Attribute amp not allowed on element html at this point.

I've been digging around various specs and can't find any that reference extra bits after 'html' apart from things in the style 'x=y' where x is one of https://html.spec.whatwg.org/multipage/dom.html#global-attributes

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 7, 2015

Lets just say we are aware :)

<html ⚑> is even worse. Not even valid SGML, let alone HTML.

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 7, 2015

Just allow an attribute named "data-amp" as well

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 7, 2015

@JohannesEH That came up today and it a good idea! πŸ‘

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 7, 2015

@cramforce At least it'll make it valid html5. πŸ˜‰

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 7, 2015

HTML validators will still complain about the custom elements, though (wrongly) and stuff like placeholder on divs inside custom elements.

I don't think that is a bug in AMP.

@erwinmombay

This comment has been minimized.

Copy link
Member

erwinmombay commented Oct 7, 2015

/me facepalm, i thought we had already allowed it. totally misread.

@stevefaulkner

This comment has been minimized.

Copy link

stevefaulkner commented Oct 8, 2015

Conformance checking tools lag behind UA implementations, in particular there are issues for conformance checkers to support custom attributes (while still emitting useful errors for incorrect use of other attributes). This may be a helpful read to put HTML conformance issues in perpsective - HTML5 – Check it Before you Wreck it with Mike[tm] Smith. Also note that https://validator.w3.org/nu/ provides a UI (in results view) for custom supression of errors/warning

@kevinmarks

This comment has been minimized.

Copy link

kevinmarks commented Oct 8, 2015

is also worse because you're not supposed to put in non-ascii until you hit an encoding declaration.
@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 8, 2015

O(fun) is important.
I like the ⚑
https://twitter.com/cramforce/status/651769139204259840

I think adding data-amp is OK, although adding complexity to the spec must be taken with lots of thought. Currently leaning no. AMP reserves the right to introduce attribute names.

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 8, 2015

@cramforce - IMO it really won't be the "data-amp" attribute which will add any complexity or headaches for the maintainers and users of the AMP library in the future... It will most likely be the "amp" and "⚑" attributes. Personally, I would strongly consider removing one or both of the non-standard attributes before the final 1.0 release, while you still don't have to consider backwards compat. A lot exactly to keep down complexity.

I mean I don't particularly like the data-amp attribute option, and it's possible there is another/better/fun way of expressing the amp intent in the html element, maybe by including a class name such as

<html class="⚑">...</html>

or

<html class="amp">...</html>

Both are valid HTML5...

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 9, 2015

Using class doesn't seem like a good idea to me.

I think introducing new attribute names is fine and unproblematic. We do
this in lots of other places (e.g. placeholder).

What are actual practical issues with the "amp" attribute? AMP files will
never validate in a vanilla HTML validator.

On Thu, Oct 8, 2015 at 4:55 PM Johannes Hansen notifications@github.com
wrote:

@cramforce https://github.com/cramforce - IMO it really won't be the
"data-amp" attribute which will add any complexity or headaches for the
maintainers and users of the AMP library in the future... It will most
likely be the "amp" and "⚑" attributes. Personally, I would strongly
consider removing one or both of the non-standard attributes before the
final 1.0 release, while you still don't have to consider backwards compat.
A lot exactly to keep down complexity.

I mean I don't particularly like the data-amp attribute option, and it's
possible there is another/better/fun way of expressing the amp intent in
the html element, maybe by including a class name such as

...

or

...

Still HTML5 compliant...

β€”
Reply to this email directly or view it on GitHub
#472 (comment).

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 9, 2015

@cramforce - I guess it depends on the level of browser compatibility you guys are aiming for with AMP. I fear using non-standard html might make it harder to guarantee compatibility on a broad scale.

As far as I can tell the only part of AMP that isn't valid html5, is the use of invalid attribute names (ie. non "data-" prefixed names). Personally I think that is a "practical issue" which might lead to unforeseen issues or complexity down the road.

My question to you is, why wouldn't you want to try and conform to the HTML5 standard?

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 9, 2015

@JohannesEH Documented here https://github.com/ampproject/amphtml/blob/master/DEVELOPING.md#supported-browsers

There are, however, no known issues with browsers and custom attribute names.

The general question: Why use non-standard attribute names like "placeholder"? Better ergonomics.

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 9, 2015

@cramforce - Fair enough

However, I reserve the right to say "I told you so" if any issues do creep up in the future. πŸ˜‰

@cramforce

This comment has been minimized.

Copy link
Member

cramforce commented Oct 9, 2015

@JohannesEH I'm wrong all the time :)

Just glad we didn't make it <html &> :)

@JohannesEH

This comment has been minimized.

Copy link

JohannesEH commented Oct 9, 2015

@cramforce - Ha ha, yeah me too. Nah just call it <html data-&amp;> and you should be fine. 😜

#notreally

@donohoe

This comment has been minimized.

Copy link

donohoe commented Oct 9, 2015

I believe the continued usage of ⚑is unwise and should be removed from the spec immediately.

It seems the primary reason to have it is that it just looks good. That aside, its not something that is helpful if you're searching Github, Google, StackOverflow and likely many other search engines.

Its also not something you can easily type. Sure there is "amp" but why have 2 ways to do the same thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment