Skip to content

Conversation

@martinthomson
Copy link
Contributor

As far as I can tell, the code that was supposed to rewrite references was inoperative. It just glued the same URL back together. This version is effective.

The links to RFCs in the header of documents were being missed. I did not change the RFC link that appears in boilerplate.

I also update the text of the link if it is the same as the destination, to avoid surprises.

This also applies the fix unconditionally, so that the treatment is consistent regardless of what preference is chosen.

As far as I can tell, the code that was supposed to rewrite references
was inoperative.  It just glued the same URL back together.  This
version is effective.

The links to RFCs in the header of documents were being missed.
I did not change the RFC link that appears in boilerplate.

I also update the text of the link if it is the same as the destination,
to avoid surprises.

This also applies the fix unconditionally, so that the treatment is
consistent regardless of what preference is chosen.
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.54%. Comparing base (ff75680) to head (e954040).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10022      +/-   ##
==========================================
- Coverage   88.55%   88.54%   -0.01%     
==========================================
  Files         317      317              
  Lines       42343    42385      +42     
==========================================
+ Hits        37496    37530      +34     
- Misses       4847     4855       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@martinthomson
Copy link
Contributor Author

@larseggert, this is another one of your changes. Maybe you can help tell if this makes sense.

Copy link
Collaborator

@larseggert larseggert left a comment

Choose a reason for hiding this comment

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

If it works now, it makes sense :-)

We really need unit tests for all this datatracker JS code.

document.activeElement.blur();

// keep RFC links within the datatracker
const rfclink = /^https?:\/\/[^\/\.]*.(?:rfc-editor|ietf).org\/.*\/((?:rfc|std|bcp)\d+)$/;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps should limit the subdomains that are treated as RFC links here. A link to https://notes.ietf.org/.../bcp7890 isn't an RFC link. It's also not a great example as it's not likely to appear at all, but the point is real RFC links are at a more constrained set of hosts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't confident that I could enumerate the locations. Is it just www.ietf.org and datatracker.ietf.org and www.rfc-editor.org? The deployment architecture of this stuff (and its history) are a bit opaque to me.

Copy link
Member

Choose a reason for hiding this comment

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

I'll have a look, but imo it'd be better to start off conservatively

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.

3 participants