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

Allow anchor links containing colon #87

Closed

Conversation

heathd
Copy link
Contributor

@heathd heathd commented Nov 22, 2013

A colon is a legitimate character to contain in an ID reference in an
anchor link[1]. The REGEX_PROTOCOL was too general and would consider a
sequence such as '#foo:1' or '/somepage#foo:1' to define the '#foo'
protocol.

The '#' character is explicitly excluded from being part of URI
scheme[2]:

  scheme        = alpha *( alpha | digit | "+" | "-" | "." )

Therefore it seems safe to exclude the character from the initiali part of REGEX_PROTOCOL.

[1] http://www.w3.org/TR/html401/types.html#type-name
[2] http://www.ietf.org/rfc/rfc2396.txt

A colon is a legitimate character to contain in an ID reference in an
anchor link[1]. The REGEX_PROTOCOL was too general and would consider a
sequence such as '#foo:1' or '/somepage#foo:1' to define the '#foo'
protocol.

The '#' character is explicitly excluded from being part of URI
scheme[2]:

      scheme        = alpha *( alpha | digit | "+" | "-" | "." )

[1] http://www.w3.org/TR/html401/types.html#type-name
[2] http://www.ietf.org/rfc/rfc2396.txt
@rgrove
Copy link
Owner

rgrove commented Nov 22, 2013

Thanks for the excellent PR! I've merged this into the dev-2.1.0 branch. It'll make its way to master when 2.1.0 is released.

@rgrove rgrove closed this Nov 22, 2013
@heathd
Copy link
Contributor Author

heathd commented Nov 22, 2013

👍 great

kalleth added a commit to alphagov/govspeak that referenced this pull request Aug 1, 2014
The older version used by govspeak (2.0.3) contains a bug where colons in
ids would not be allowed, which prevents footnotes being saved as part
of a document body in specialist-publisher.

This was fixed in 2.1.0 (see
rgrove/sanitize#87).

Trying to force an upgraded version of sanitize to be activated using
the specialist-publisher Gemfile is a non-starter as bundle refuses to
upgrade to that version as it's too far ahead of Govspeak's
requirements.
kalleth added a commit to alphagov/govspeak that referenced this pull request Aug 4, 2014
The older version used by govspeak (2.0.3) contains a bug where colons in
ids would not be allowed, which prevents footnotes being saved as part
of a document body in specialist-publisher.

This was fixed in 2.1.0 (see
rgrove/sanitize#87).

Trying to force an upgraded version of sanitize to be activated using
the specialist-publisher Gemfile is a non-starter as bundle refuses to
upgrade to that version as it's too far ahead of Govspeak's
requirements.
kalleth added a commit to alphagov/govspeak that referenced this pull request Aug 4, 2014
The older version used by govspeak (2.0.3) contains a bug where colons in
ids would not be allowed, which prevents footnotes being saved as part
of a document body in specialist-publisher.

This was fixed in 2.1.0 (see
rgrove/sanitize#87).

Trying to force an upgraded version of sanitize to be activated using
the specialist-publisher Gemfile is a non-starter as bundle refuses to
upgrade to that version as it's too far ahead of Govspeak's
requirements.
mililk4 added a commit to mililk4/govspeak that referenced this pull request Jul 1, 2024
The older version used by govspeak (2.0.3) contains a bug where colons in
ids would not be allowed, which prevents footnotes being saved as part
of a document body in specialist-publisher.

This was fixed in 2.1.0 (see
rgrove/sanitize#87).

Trying to force an upgraded version of sanitize to be activated using
the specialist-publisher Gemfile is a non-starter as bundle refuses to
upgrade to that version as it's too far ahead of Govspeak's
requirements.
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.

2 participants