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

Link to attribute value goes to an unrelated definition #4435

Open
mgiuca opened this issue Apr 24, 2023 · 1 comment
Open

Link to attribute value goes to an unrelated definition #4435

mgiuca opened this issue Apr 24, 2023 · 1 comment
Labels

Comments

@mgiuca
Copy link

mgiuca commented Apr 24, 2023

Description of problem

A link to an attribute value is being targeted at an unrelated definition with the same name.

From within the Manifest spec: [^link/rel/manifest^] should link to #link-type-manifest in the HTML spec, but instead links to #dfn-manifest in itself.

URL to affected spec or repo:

https://w3c.github.io/manifest/#updating - see the first sentence there:

As specified for [^link/rel/manifest^] link relation...

What happened (e.g., it crashed)?:

It links to #dfn-manifest, seemingly ignoring the explicit "link/rel" part and just trying to find a definition named "manifest".

Expected behavior (e.g., it shouldn't crash):

It should find the attribute value "manifest" for the attribute "rel" of the element "link", which is defined in HTML at #link-type-manifest.

This works properly for any other type of link, e.g., [^link/rel/stylesheet^]. I think it's getting distracted by the fact that we have a definition of "manifest" in this same document.

See, Respec itself tells me to cite it using this syntax (and it says to use [=manifest=] to get the other definition):

image

@mgiuca mgiuca added the bug label Apr 24, 2023
@sidvishnoi
Copy link
Member

I think it's getting distracted by the fact that we have a definition of "manifest" in this same document.

Indeed. Following is how we try to link to external definitions:

https://github.com/w3c/respec/blob/main/src/core/link-to-dfn.js#L77-L89

I'll see if we can make the explicit one take priority (perhaps adding a data-cite somewhere). We only do two passes on dfn referencing: local first, and then external - this is unlikely to change.

mgiuca added a commit to mgiuca/manifest that referenced this issue Apr 24, 2023
(Actually this makes the wrong link, but it's a respec bug: w3c/respec#4435
marcoscaceres added a commit to w3c/manifest that referenced this issue May 2, 2023
… document (#1069)

* Processing the manifest: Simplify the interface.

Replaces the link and response parameters with document URL and manifest URL.

These parameters were only used to get the document URL and manifest
URL, respectively, so it doesn't make sense to accept the much bulkier
HTML objects. This was limiting the ability to call the processing
algorithm from outside an HTML document context, which is a future
direction we wish to explore.

Note that the only call to this algorithm is in the HTML spec, which
needs to be updated simultaneously to use the new interface.

Pre-work for #668.

* Added new normative text (with a non-normative note) allowing user
agents to invoke the processing steps without a document, provided that
they supply a valid document URL.

* Reword non-normative note.

* Correctly link.

(Actually this makes the wrong link, but it's a respec bug: w3c/respec#4435

* Added a SHOULD to set CORS correctly.

Note: There's a reference error here because HTML doesn't export a term. I'm getting it exported.

* Move all this text to its own section; it's getting a bit much.

* Use variables to avoid repeating complex sentences.

* Rewrote processing without a document section for clarity and correctness.

- Changed MUST into a SHOULD. We can't really expect all uses to
  directly verify this.
- Removed the "or" clause that the document be same-origin as manifest;
  you still want a link from the document to the manifest either way.
- Added "at least at some point in the past", to acknowledge that you
  don't need to verify this at install time, just whenever you did the
  caching.
- Clarify that the CORS request is only needed if the manifest is not
  same-origin as the document.

* Apply suggestions from code review

Co-authored-by: Marcos Cáceres <marcos@marcosc.com>

* Respond to review.

---------

Co-authored-by: Marcos Cáceres <marcos@marcosc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants