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

Inconsistent index escaping #2535

Open
0branch opened this issue Jan 3, 2019 · 7 comments
Open

Inconsistent index escaping #2535

0branch opened this issue Jan 3, 2019 · 7 comments
Assignees
Labels
big Issue consisting of many subissues

Comments

@0branch
Copy link

0branch commented Jan 3, 2019

Problem

The current escaping rules seem to be inconsistent between anchor names and corresponding index entries.

See [df15cce] for the latest example of this:

  • Anchor name on the page: index-entry-declarator_blocks_%23|
    • Index entry: #index-entry-declarator_blocks_%2523%7C
  • Anchor name on the page: index-entry-declarator_blocks_%23=
    • Index entry: #index-entry-declarator_blocks_%2523%3D

Chrome and Safari can resolve these links while Firefox apparently cannot (i.e., clicking #| in the search dropdown will take the user to the top of /language/pod rather than to the anchored declarator block section).

Suggestion

Ensure that the escaping used for anchors is also used in the index (i.e., address the inconsistency). In the above, page anchors should use <%7C %3D> rather than unescaped <| =>.

@JJ
Copy link
Contributor

JJ commented Jan 3, 2019

Please, @cfa , can you check this out?
At any rate, well, let's say that all the indexing thing is less than optimal. Thanks for the report.

@cfa cfa self-assigned this Jan 4, 2019
@cfa
Copy link
Contributor

cfa commented Jan 4, 2019

Sure, I'll take a look.

@cfa
Copy link
Contributor

cfa commented Jan 4, 2019

First pass: it looks like rewrite-url-logged is called from write-search-file; this is what's mapping
#index-entry-declarator_blocks_%23= to #index-entry-declarator_blocks_%2523%3D via rewrite-url in Pod::Htmlify.

The anchor for the X<> ref (containing a literal '=') is generated by pod2html (exported by Pod::To::HTML). So, it seems that the problem is divergent escaping logic in rewrite-url vs. pod2html.

Will keep digging.

@cfa
Copy link
Contributor

cfa commented Jan 4, 2019

Re: escaping, I think there are two related issues:

  1. rewrite-url escapes = as %3D; pod2html does not
  2. rewrite-url is escaping already escaped URLs—%23 becomes %2523

Amusingly, It looks like %23%3D works in Firefox but not Chrome or Safari—so both these points need to be addressed for browser compatibility.

RfC @tbrowder @JJ (recent contributors to Pod::Htmlify).

@JJ
Copy link
Contributor

JJ commented Jan 4, 2019

Yep, I've been there and arrived to the point where it's easier to ditch it all and start all over again than keep patching it, because URLs are being generated, and then sent back and forth between pod and HTML, acquiring stuff on the process. Simply, the anchor generating part has to be rewritten, period. Actually, #1823 says the whole thing has to be rewritten.
So I would say let's acknowledge this, but let's leave it in the freezer for some time waiting major refactoring (which @finanalyst is doing) or rewriting of major parts of it.

@JJ JJ added build big Issue consisting of many subissues labels Jan 4, 2019
@JJ JJ mentioned this issue Jan 18, 2019
antoniogamiz added a commit to Raku/Pod-To-HTML that referenced this issue Sep 1, 2019
@2colours
Copy link
Contributor

2colours commented Mar 3, 2023

Seems like this is not relevant to this repository anymore. Honestly, I can't even find these index-prefixed anchors, although I remember them. In any case - if there are problems, they should be consolidated under https://github.com/Raku/doc-website/.

@cfa
Copy link
Contributor

cfa commented Mar 3, 2023

Here are a few matches for index-entry.*% in the doc tree; the first few links I tried in the rendered site seem to be incorrect.

@2colours++ regarding escaping being the responsibility of the doc-website repo. Use of incorrect/broken index anchors in L<> links remains relevant here though. Might warrant a new issue?

@coke coke removed the build label Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big Issue consisting of many subissues
Projects
None yet
Development

No branches or pull requests

6 participants