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

Meta: add link-type-alternate as ID for rel=alternate #5855

Merged
merged 1 commit into from Aug 30, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented Aug 24, 2020

The former W3C fork changed the rel-alternate ID and now links to there redirect to the HTML Standard and end up leading nowhere.

See protocol-registries/link-relations#22.


/links.html ( diff )

@annevk
Copy link
Member Author

annevk commented Aug 24, 2020

We use empty span elements in headings elsewhere, but I guess they do not go together with the dfn? I can also put the ID on the dfn but I think that would change the internal links. Thoughts?

@sideshowbarker
Copy link
Contributor

I just tested locally after dropping the code from wattsi which generates that error. Here’s the output:

  <h5 id=rel-alternate><span class=secno>4.6.6.1</span> <span
  id=link-type-alternate></span>Link type "<dfn
  data-dfn-for=link/rel,a/rel,area/rel data-dfn-type=attr-value
  data-export=""><code>alternate</code></dfn>"<a href=#rel-alternate
  class=self-link></a></h5>

…which seems to look fine. And that makes the dfn popup behavior work just fine too, so it doesn’t regress anything afaict.

So I kinda left wondering why that check/error were added to wattsi to begin with.

Regardless, it seems like we could fix this just by dropping the conditional in the wattsi sources which is causing it.

sideshowbarker added a commit to whatwg/wattsi that referenced this pull request Aug 28, 2020
This change drops an error that Wattsi emits for the case where a
heading has an empty span or dfn (that is, with no text content),
followed by another span or dfn in the same heading.

The current code appears to have been (for some reason that’s not
obvious now) very intentionally written to report an error for exactly
that case — but not for the case where of a heading with *non-empty*
span or dfn followed by another span or dfn in the same heading.

Without this case, Wattsi reports an error for a reasonable markup case
like the one in whatwg/html#5855; that is, this:

<h5 id="rel-alternate"><span id="link-type-alternate"></span>Link type "<dfn data-export=""
data-dfn-for="link/rel,a/rel,area/rel" data-dfn-type="attr-value"><code
data-x="rel-alternate">alternate</code></dfn>"</h5>

It’s not at all obvious why we’d want a case like that to be an error.
sideshowbarker added a commit to whatwg/wattsi that referenced this pull request Aug 28, 2020
This change drops an error that Wattsi emits for the case where a
heading has an empty span or dfn (that is, with no text content),
followed by another span or dfn in the same heading.

The current code appears to have been (for some reason that’s not
obvious now) very intentionally written to report an error for exactly
that case — but not for the case where of a heading with *non-empty*
span or dfn followed by another span or dfn in the same heading.

Without this change, Wattsi reports an error for a reasonable markup case
like the one in whatwg/html#5855; that is, this:

<h5 id="rel-alternate"><span id="link-type-alternate"></span>Link type "<dfn data-export=""
data-dfn-for="link/rel,a/rel,area/rel" data-dfn-type="attr-value"><code
data-x="rel-alternate">alternate</code></dfn>"</h5>

It’s not at all obvious why we’d want a case like that to be an error.
sideshowbarker added a commit to whatwg/wattsi that referenced this pull request Aug 29, 2020
This change drops an error that Wattsi emits for the case where a
heading has an empty span or dfn (that is, with no text content),
followed by another span or dfn in the same heading.

The current code appears to have been (for some reason that’s not
obvious now) very intentionally written to report an error for exactly
that case — but not for the case where of a heading with *non-empty*
span or dfn followed by another span or dfn in the same heading.

Without this change, Wattsi reports an error for a reasonable markup case
like the one in whatwg/html#5855; that is, this:

<h5 id="rel-alternate"><span id="link-type-alternate"></span>Link type "<dfn data-export=""
data-dfn-for="link/rel,a/rel,area/rel" data-dfn-type="attr-value"><code
data-x="rel-alternate">alternate</code></dfn>"</h5>

It’s not at all obvious why we’d want a case like that to be an error.
The former W3C fork changed the rel-alternate ID and now links to there redirect to the HTML Standard and end up leading nowhere.

See protocol-registries/link-relations#22.
@annevk
Copy link
Member Author

annevk commented Aug 29, 2020

So PR Preview still fails somehow. Could that be because some server component isn't updated or something else?

@sideshowbarker
Copy link
Contributor

So PR Preview still fails somehow. Could that be because some server component isn't updated or something else?

Yeah, I vaguely recall it breaking before in a case like this — but don’t recall who was able to fix it. As far as I know, I don’t myself have access to the shell environment or config UI for it.

@domenic
Copy link
Member

domenic commented Aug 29, 2020

Sorry, yes, I always need to SSH into wattsi-server and re-update it manually. I should really automate that.

@annevk annevk merged commit 5680d80 into master Aug 30, 2020
@annevk annevk deleted the annevk/link-type-alternate branch August 30, 2020 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants