Skip to content
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

Use id attribute instead of name for ToC #111

Closed
wants to merge 1 commit into from
Closed

Use id attribute instead of name for ToC #111

wants to merge 1 commit into from

Conversation

josh
Copy link
Contributor

@josh josh commented Feb 6, 2014

The name attribute is no longer considered a global attribute or part of the a element as of the XHTML/HTML5 spec. id has replaced this deprecated usage of the name attribute. name is now reserved only for "form controls".

Note that in XHTML 1.0, the name attribute of these elements is formally deprecated, and will be removed in a subsequent version of XHTML. - http://www.w3.org/TR/xhtml1/#h-4.10

Related

cc @jch @raganwald @mislav

id = text.downcase
id.gsub!(PUNCTUATION_REGEXP, '') # remove punctuation
id.gsub!(' ', '-') # replace spaces with dash
id = EscapeUtils.escape_uri(id)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoping @jakedouglas or @brianmario can review this change. The escape utils called was removed in #64. It seems like nokogiri did some magic with the name attribute, but won't for the id.

@bkeepers
Copy link
Contributor

bkeepers commented Feb 6, 2014

Oh dang! That is really unfortunate. This is going to cause trouble with user content clashing with IDs used by the app. See github/markup#219 (comment)

Does HTML5 define any way for creating anchors to user-generated content without the potential of conflicting with the app? IMHO, id should really be reserved for implementors and not user-generated content.

@josh
Copy link
Contributor Author

josh commented Feb 6, 2014

Gah, I really wish we could prefix these.

Another issue with name is that in IE, all the references are exported to the window object. So if you do <a name=foo>. window.foo will reference that element.

This can introduce other JS bugs. Mainly around feature detecting. Say you added a # Promise header to your md, you'd actually break the Promise js polyfill.

@josh
Copy link
Contributor Author

josh commented Feb 6, 2014

Still can't merge as is. We're discussing some sort of prefixing solution.

@raganwald
Copy link

Another issue with name is that in IE, all the references are exported to the window object. So if you do <a name=foo>. window.foo will reference that element.

This can introduce other JS bugs. Mainly around feature detecting. Say you added a # Promise header to your md, you'd actually break the Promise js polyfill.

Why oh why oh why hasn't IE died in a 🔥 yet?

@josh
Copy link
Contributor Author

josh commented Feb 6, 2014

Closing for now. Thinking about a different approach.

@josh josh closed this Feb 6, 2014
@mislav
Copy link
Contributor

mislav commented Feb 6, 2014

Yeah we can't let people inject IDs in the page. But we also can't break everyones URLs w/ fragment identifiers by prefixing all the headings. If we prefix, we need to handle old fragment identifiers in JS and perform scrolling manually. Not sure I want us to go down that road—another browser feature reinvented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants