Comments (5)
This is a long standing issue in the GitHub markup pipeline, so much so that I think this should conclude with at least a statement in the README as to why GitHub does what they do.
Intuition tells me that GitHub does it this way to avoid conflicts with CSS in user content. The challenge is that a markup file can have absolutely any heading, which means any anchor name. If assigned as an id attribute, there's a high likelyhood that one of the headings is going to match an id selector in the GitHub stylesheet and get styled in unexpected ways.
Watching GitHub work over the last few years, I've seen several strategies applied here to avoid conflicts, from dropping anchors outright to prefixing them with something like "user-content-". Every change seems to break something different.
From what I can see, the name
is a reasonable compromise (though I'm not saying ideal). At least in Chrome and Firefox, the name attribute allows the anchor point to work. I'm sure there are browsers that don't support it, but it would be incorrect to say it doesn't work at all.
More transparency around decision making would be very welcomed here. I do understand the challenge that GitHub faces trying to avoid conflicts with open-ended content. Unless we all enjoy seeing the same issue get filed once a month, perhaps it's time to document the reason for why it is the way it is.
In other words, answer the question, "Why do we define heading anchors using the name
attribute instead of the id
attribute?"
from html-pipeline.
To give some context and address the concerns that @mojavelinux raised…
TL;DR: Ideally, browsers would implement a way to deep-link to elements that doesn't clobber the DOM and have a negative impact on CSS and JavaScript, but they don't, so we've had to come up with workarounds.
We originally started sanitizing id
to guard against user content clobbering elements used by CSS or JavaScript. For example, if we allowed <a id="new_repository">…</a>
through, it would break the "New Repository" link on that page for any JavaScript-enabled browser. Allowing name
was a less-than-ideal-but-acceptable workaround for enabling deep-linking within user content…until we realized that the name
attribute clobbers the DOM. For example, <a name="addEventListener">…</a>
would break event handling in JavaScript.
So a few months ago, we rolled out a change that prefixed all name
elements with user-content-
, and then use JavaScript to perform the actual scrolling to user content. For example <a name="example"></a>
gets transformed into <a name="user-content-example"></a>
, and whenever a page is loaded or the hash changes (e.g. https://github.com/jch/html-pipeline/issues/135#example
), JavaScript looks for prefixed user content that matches the hash and scrolls to it.
While this again is not a perfect solution, but it has worked really well. It allows users to deep link to content without DOM clobbering. Using a similar technique, I think we can go back to allow id
in user content.
See #111 and github/markup#219 (comment) for related discussions.
/cc @josh
from html-pipeline.
I think adding back id support with that prefix would be legit. Pretty much address any security concerns around breaking QSA and overriding application defined behaviors built on ids.
from html-pipeline.
For example,
<a name="addEventListener">…</a>
would break event handling in JavaScript.
@bkeepers, what browsers does this issue come up in?
I face the similar problem with anchors now, and <a name="foo">
doesn't add anything to DOM in recent Chrome and FF versions I tested, only <form name="foo">
does that.
from html-pipeline.
@rlidwka I think I experienced it in both Firefox and Chrome a few months ago. Maybe that has since changed. Our security team was able to make a page completely unusable by adding content with name
attributes (I don't remember if that was addEventListener
or something else).
from html-pipeline.
Related Issues (20)
- 2.14.0 is disconnected HOT 4
- Allow `loading` attribute on images HOT 5
- Since bump 2.14.2 builds are failing HOT 3
- Allow vertical-align HOT 1
- Indicate a version for activesupport that has support/receives security patches (>= 6?) HOT 2
- v3: Question regarding requiring a ConvertFilter if there are NodeFilters HOT 1
- v3 gemoji, gemojione seem required - is there a way to not require at puma startup? HOT 2
- Suggestion: add more tags to the sanitization filter HOT 3
- So what DOES GitHub use now? HOT 1
- ActionView::Template::Error with version 3.0.0 HOT 8
- Bug in specification of node filters in v3.0.0 HOT 1
- convert_filter only executed when node_filters are present HOT 3
- Can't seem to get a <script> to run when added to the allowlist? HOT 2
- Error when passing instance of `TextFilter` class to `text_filters` option HOT 3
- Context & result of filters except for text_filters aren't overwritten on call time HOT 3
- Sanitizing inline style attributes HOT 2
- Question about original and new SanitizationFilter HOT 2
- Why are node_filters applied twice? HOT 3
- Update to 3.3.4? HOT 2
- v3.2.1 is badly broken, I think because of PR #408 HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from html-pipeline.