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.

Sign up for GitHub

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

Jump to bottom

Look for duplicate exported dfns #306

Open
tidoust opened this issue Jul 15, 2021 · 10 comments
Open

Look for duplicate exported dfns #306

tidoust opened this issue Jul 15, 2021 · 10 comments

Comments

@tidoust
Copy link
Member

tidoust commented Jul 15, 2021

Some specs define and export terms that are defined elsewhere, see e.g. discussion in https://github.com/w3c/respec/issues/3687

There may be valid cases where this is needed. It would be good to first look at exported dfns and list terms that are defined more than once.
@marcoscaceres
Copy link
Member

See also w3c/aria#1510

@tidoust
Copy link
Member Author

tidoust commented Jul 18, 2021

Here is a first dump of exported terms defined in more than one spec. This is the result of parsing the dfns extracts with the following rules:

  1. Only consider exported dfns (dfn.access === 'public')
  2. Only consider root dfns (dfn.for.length === 0)
  3. Consider that dfns in CSS modules override the same dfns in CSS 2 and SVG 2
  4. Consider that css-align dfns override those in css-flexbox
  5. Consider that css-position dfns override those in css-logical
  6. Ignore duplicate IDL terms, we know about them and fix them already through IDL patches (although note we don't yet fix them in dfns extracts)
  7. Ignore duplicate elements in HTML and SVG 2 (the a, link, style, title elements)

These rules take care of most but not all of the duplicate CSS properties identified in #127 (comment).

Looking at the results, there are a number of cases where the duplicates are warranted (e.g. the notion of attribute in DOM is different from the notion of attribute in Web IDL). There are cases where specs need fixing (e.g. wai-aria should reference Infra for user agent, conformant server should not be exported, Trusted Types has a few weird constructs). And there are cases that are questionable (e.g. the notion of character? Why does Page Lifecycle re-define discard, is it an extended dfn?)

Duplicate dfns with same type (27 found)
Duplicate dfns with different types (54 found)

@dontcallmedom
Copy link
Member

dontcallmedom commented Aug 5, 2021 •
edited
Loading

in the list of same name, different types, there were 6 were the duplicate came from the same spec (Trusted Types), due to a bug in the markup - PR filed at w3c/trusted-types#348

re "character" in the first list, it's interesting that the visible term in Infra is actually "code point", with character only provided as an alternate linking text - maybe one that should be deprecated given the ambiguity of the term.

I'm surprised the 2nd list is so short; for instance, width is also used in HTML, SVG, as a mediastreamtrack constraint, etc. I guess these are filtered out by the "root-only dfns" rule.

@tidoust
Copy link
Member Author

tidoust commented Aug 13, 2021

Note code that produces the previous report is in a dedicated Reffy branch:
https://github.com/w3c/reffy/blob/study-dfns/src/cli/study-dfns.js

@cdoublev cdoublev mentioned this issue Aug 20, 2021
Open
@cdoublev
Copy link

I leave a note here but please tell me if you prefer that I open a dedicated issue.

<rgb()> and <hsl()> are defined with a legacy syntax that @webref/css does not extract. The legacy syntax defines a comma separated list of arguments while the modern syntax defines a whitespace/slash separated list. The legacy syntax will never be removed, I presume, and I would bet many CSS authors use the legacy syntax instead of the new one.

<!-- Markup for the modern syntax -->
<pre class="prod highlight">
    <dfn class="dfn-paneled" data-dfn-type="function" data-export="" id="funcdef-hsl">
        <c- nf="">hsl</c-><c- p="">()</c->
    </dfn>
    <!-- ... -->
</pre>
<!-- Markup for the legacy syntax -->
<pre class="prod highlight">
    <c- nf="">hsl</c-><c- p="">()</c->
    <!-- ... -->
</pre>

I'm not sure what to suggest and where to suggest it. I believe that Bikeshed should allow defining a legacy syntax that @webref/css could assign to an additional legacy for an entry in valuespace, but to my knowledge, these are the only types that have two value definitions defined in the same spec.

@tidoust tidoust mentioned this issue Apr 19, 2022
Closed
@tidoust
Copy link
Member Author

tidoust commented Apr 19, 2022

Thanks @cdoublev, I moved your comment to a dedicated issue #563.

@tidoust
Copy link
Member Author

tidoust commented Dec 16, 2022

Refreshing the list of duplicate definitions and completing with the list of IDL terms that are defined more than once (before patches are applied):

Duplicate dfns with same type (64 found)
Duplicate dfns with different types (64 found)
Duplicate IDL dfns (11 found)

@tidoust
Copy link
Member Author

tidoust commented Dec 16, 2022 •
edited
Loading

Reflecting on the duplicate IDL dfns:

  • DOMParser: definition in Trusted Types appears in an IDL block flagged with an "exclude" class. Definitions should probably not be extracted at all.
  • MessageEventSource, PermissionState: handled in IDL patch. Could be hardcoded in Reffy.
  • MutationEvent: legacy, defined as removed in DOM, and deprecated in UI Events. Could be hardcoded in Reffy.
  • RTCStats: definition in WebRTC Stats is informative (but exported).
  • EvalError, RangeError, ReferenceError, SyntaxError, TypeError, URIError: Shouldn't Web IDL reference definitions in ECMAScript rather than define the terms? Or do we want to prefer Web IDL definitions because they are closer to relevant algorithms for Web specs?

@tidoust tidoust mentioned this issue Dec 16, 2022
Merged
@dontcallmedom
Copy link
Member

RTCStats: definition in WebRTC Stats is informative (but exported).

Marking the definition as non-exported should be an easily accepted patch there

EvalError, RangeError, ReferenceError, SyntaxError, TypeError, URIError: Shouldn't Web IDL reference definitions in ECMAScript rather than define the terms? Or do we want to prefer Web IDL definitions because they are closer to relevant algorithms for Web specs?

From the discussion in w3c/reffy#732, I believe it was indeed preferred to keep WebIDL as an intermediary, although I'm not sure how deeply discussed this was.

@dontcallmedom
Copy link
Member

Duplicate dfns with same type

Some of these are different concepts altogether of terms that happen to be the same across specs (e.g. an attribute in DOM is quite different from an attribute in WebIDL); some of these are for concepts that are pretty close but not fully equivalent definitions (e.g. surrogate in Infra & I18N glossary); some of these are explicitly duplicate definitions that happen to be repeated for mostly logistical / editorial reasons (e.g. the overlap between rdf concepts & rdf semantics; the duplicates across CSS modules).

I think these 3 categories probably lead to different type of solutions:

  • for the 1st one, namespacing with a dfn-for the definitions would be an obvious candidate - not sure if that would work across the board though
  • for the 2nd one, trying to get convergence on a single definition in a single place would be the ideal approach; if not possible, trying to encourage using different terms would be an alternative
  • for the 3rd one, we would probably want a mechanism to describe this equivalence relationship - this would allow in ReSpec and Bikeshed to avoid the duplicate warning; in ReSpec, this would allow using either specs in the xref setting and still be able to use the relevant definition

This was referenced Dec 19, 2022
Merged
Open
Merged
@tidoust tidoust mentioned this issue Apr 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dontcallmedom @marcoscaceres @tidoust @cdoublev