E545 Use respec autolinks and use expected conventions to define elements … by dontcallmedom · Pull Request #179 · w3c/mathml-core · GitHub
[go: up one dir, main page]

Skip to content

Conversation

dontcallmedom
Copy link
Member

Definition conventions aligns with https://tabatkins.github.io/bikeshed/#dfn-contract (which allows for definitions to be properly extracted for use by other specifications)

Autolinks (when available) help making the spec resistant to change of anchors, and allows for instance to link to the much-more-reader-friendly multipage version of HTML.

This pull request targets minimal changes in the produced spec, but I noted the following possible additional changes worth considering:

  • I suspect the convention to use mspace@width and mpadded@width (and similar pattern for other attributes) was to avoid duplicate definitions; with this new markup, this would no longer be needed (since one unambiguously links to one or the other with [^mspace/width^] or [^mpadded/width^] ); I didn't remove the convention from the document since I wasn't sure whether there was other reasons for using that convention, but would be happy to follow up with that change
  • both the HTML and SVG specs don't enclose their element names (and definitions) within <…> - I haven't changed this in the document, but doing so would allow to simplify a bit some of the changes here (e.g. I wouldn't need to use data-lt on the element definitions).
  • I'm also not sure there is any value in re-dfning locally the global attributes that are shared with HTML. nonce, data-* and tabindex could use the same treatment if/when Rename HTMLOrSVGElement to reflect its wider use in MathML as well whatwg/html#5248 gets merged
  • I couldn't auto-links to all the HTML definitions, because they're not currently marked as exported there, e.g. "presentational hint" - it would probably be useful to start a discussion with the HTML editors on exporting those; a specific case is that MathML links to HTML for the definition of "reflects" (where it's not exported), whereas an equivalent and exported definition exists in the DOM spec - I'm happy to make that change unless there was again a specific reason to use the HTML link
  • the terms defined in CSS colors, CSS Box & CSS writing modes could also be auto-linked to, but only by switching to level 4 instead of 3; I wasn't sure if picking 3 was an explicit choice, or just what was available when the links were initially made; I'm happy to update the PR with the autolinks to v4 if that wasn't particularly intentional
  • the links to the visibility target CSS 2.1 at the moment in the spec; if they were to target the equivalent in CSS 2.2, or in CSS Display, they could be replaced with autolinks
  • the link for foreignObject could be turned into an autolink once Impossible to link to camelCase elements speced/respec-web-services#365 is fixed

…& attributes

Definition conventions aligns with https://tabatkins.github.io/bikeshed/#dfn-contract (which allows for definitions to be properly extracted for use by other specifications)
Autolinks (when available) help making the spec resistant to change of anchors, and allows for instance to link to the much-more-reader-friendly multipage version of HTML
@fred-wang
Copy link
Contributor

Thanks for working on this, I'm happy to rubber-stamp and merge this if you think it's ready. Some comments inline.

* I suspect the convention to use `mspace@width` and  `mpadded@width` (and similar pattern for other attributes) was to avoid duplicate definitions; with this new markup, this would no longer be needed (since one unambiguously links to one or the other with `[^mspace/width^] ` or `[^mpadded/width^] `); I didn't remove the convention from the document since I wasn't sure whether there was other reasons for using that convention, but would be happy to follow up with that change

Yes, that was the only reason. Just using 'width' unambiguously would be great.

* both the HTML and SVG specs don't enclose their element names (and definitions) within `<…>` - I haven't changed this in the document, but doing so would allow to simplify a bit some of the changes here (e.g. I wouldn't need to use `data-lt` on the element definitions).

I believe we can try to align with SVG and HTML here. I'm not sure, but it's possible that this was again used to avoid duplicate definitions for e.g. <none> (that we just removed in #173) or <math>, where similar keywords exist elsewhere.

* I'm also not sure there is any value in re-`dfn`ing locally the global attributes that are shared with HTML. `nonce`, `data-*`  and `tabindex` could use the same treatment if/when [Rename HTMLOrSVGElement to reflect its wider use in MathML as well whatwg/html#5248](https://github.com/whatwg/html/pull/5248) gets merged

Agree.

* I couldn't auto-links to all the HTML definitions, because they're not currently marked as [exported](https://tabatkins.github.io/bikeshed/#dfn-export) there, e.g. "presentational hint" - it would probably be useful to start a discussion with the HTML editors on exporting those; a specific case is that MathML links to HTML for the definition of "reflects" (where it's not exported), whereas an equivalent and exported definition exists in the DOM spec - I'm happy to make that change unless there was again a specific reason to use the HTML link

Agree about "presentational hint". Switching to DOM spec for reflects sounds fine, why are there duplicate definitions for HTML and DOM?

* the terms defined in CSS colors, CSS Box & CSS writing modes could also be auto-linked to, but only by switching to level 4 instead of 3; I wasn't sure if picking 3 was an explicit choice, or just what was available when the links were initially made; I'm happy to update the PR with the autolinks to v4 if that wasn't particularly intentional

Switching to level 4 sounds good to me.

* the links to the `visibility` target CSS 2.1 at the moment in the spec; if they were to target the equivalent in CSS 2.2, or in CSS Display, they could be replaced with autolinks

Switching to CSS Display sounds good to me.

* the link for `foreignObject` could be turned into an autolink once [Impossible to link to camelCase elements respec-web-services#365](https://github.com/w3c/respec-web-services/issues/365) is fixed

OK.

@dontcallmedom
Copy link
Member Author

thanks for the quick review! I've pushed a few more commits that implement the proposed additional changes that are ready to use. I believe the PR is in good shape to be merged at this stage.

It leaves the following items as to be done potentially at a later stage:

  • I'm also not sure there is any value in re-dfning locally the global attributes that are shared with HTML. nonce, data-* and tabindex could use the same treatment if/when Rename HTMLOrSVGElement to reflect its wider use in MathML as well Rename HTMLOrSVGElement to reflect its wider use in MathML as well whatwg/html#5248 gets merged
  • I couldn't auto-links to all the HTML definitions, because they're not currently marked as exported there, e.g. "presentational hint" - it would probably be useful to start a discussion with the HTML editors on exporting those;
  • the link for foreignObject could be turned into an autolink once Impossible to link to camelCase elements respec-web-services#365 is fixed

With regard to your question:

Switching to DOM spec for reflects sounds fine, why are there duplicate definitions for HTML and DOM?

I assume it's an editorial bug.

@fred-wang fred-wang merged commit 71befa4 into w3c:main Nov 30, 2022
github-actions bot added a commit that referenced this pull request Nov 30, 2022
#179)

SHA: 71befa4
Reason: push, by fred-wang

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
dontcallmedom added a commit to dontcallmedom/html that referenced this pull request Dec 1, 2022
While it doesn't export it, the HTML spec currently redefines the 'reflect' concept, which creates possibly confusing duplication with the definition in the DOM spec. This proposes to use instead the DOM spec definition.

This was noticed in w3c/mathml-core#179
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
0