Skip to content

[css-box-4][css-backgrounds-4][css-shapes-1] Cleanup <*-box> definitions#9505

Merged
tabatkins merged 6 commits intow3c:mainfrom
cdoublev:pr-9505
Nov 10, 2023
Merged

[css-box-4][css-backgrounds-4][css-shapes-1] Cleanup <*-box> definitions#9505
tabatkins merged 6 commits intow3c:mainfrom
cdoublev:pr-9505

Conversation

@cdoublev
Copy link
Collaborator

@cdoublev cdoublev commented Oct 20, 2023

Follow-up on 7dc439c, fixing speced/bikeshed#2679.

The commit removed the <box> production (which became undefined) and replaced <box> references with <visual-box>, but some <box> references remained in other specs (including CSS Box, which defines <visual-box>).

@cdoublev
Copy link
Collaborator Author

cdoublev commented Oct 20, 2023

Hmm, <visual-box>/fill-box and other links are incorrect. I am not sure how to fix them, except by re-introducing <box> = <visual-box> | <layout-box> | ....

@cdoublev cdoublev marked this pull request as draft October 20, 2023 10:01
@cdoublev
Copy link
Collaborator Author

cdoublev commented Oct 20, 2023

Ok, I reverted the namespace change, and introduced an informal <box> definition (without a production rule).

I also simplified the production rules to also fix the namespaces defined for each individual keyword.

CSS Shapes 1 defines <shape-box> with the same keywords than <layout-box>. I leave it up to the spec authors to replace it with <layout-box>, as well as in CSS Masking 1, which includes <shape-box> in <geometry-box>.

As a recap with flattened production rules:

  <visual-box> = content-box | padding-box | border-box
  <layout-box> = content-box | padding-box | border-box | margin-box
   <shape-box> = content-box | padding-box | border-box | margin-box
<geometry-box> = content-box | padding-box | border-box | margin-box | fill-box | stroke-box | view-box
   <paint-box> = content-box | padding-box | border-box | fill-box | stroke-box
   <coord-box> = content-box | padding-box | border-box | fill-box | stroke-box | view-box

Proposed:

  <visual-box> = content-box | padding-box | border-box
  <layout-box> = <visual-box> | margin-box
   <shape-box> = <visual-box> | margin-box
<geometry-box> = <shape-box> | fill-box | stroke-box | view-box
  <paint-box>  = <visual-box> | fill-box | stroke-box
  <coord-box>  = <paint-box> | view-box

Ideally (imo):

  <visual-box> = content-box | padding-box | border-box
  <layout-box> = <visual-box> | margin-box
   <paint-box> = <visual-box> | fill-box | stroke-box
   <coord-box> = <paint-box> | view-box
<geometry-box> = <layout-box> | fill-box | stroke-box | view-box

@cdoublev cdoublev marked this pull request as ready for review October 20, 2023 12:44
@cdoublev cdoublev changed the title [css-box-4][css-backgrounds-4][css-shapes-1] Replace <box> definition with <visual-box> [css-box-4][css-backgrounds-4][css-shapes-1] Cleanup <*-box> definitions Oct 20, 2023
@w3cbot
Copy link

w3cbot commented Oct 20, 2023

svgeesus marked as non substantive for IPR from ash-nazg.

@tabatkins
Copy link
Member

tabatkins commented Oct 20, 2023

Your for values are broken. Note that Bikeshed doesn't actually process for values in any way, they're just opaque strings; the fact that <coord-box> includes <visual-box> doesn't automatically make the values that are for="<visual-box>" also for <coord-box>, etc.

So you need to accumulate all the <*-box> variants that include each keyword in the for values for that keyword.

@cdoublev
Copy link
Collaborator Author

cdoublev commented Oct 22, 2023

the fact that <coord-box> includes <visual-box> doesn't automatically make the values that are for="<visual-box>" also for <coord-box>, etc.

I think this is the way w3c/reffy will process it, assuming <visual-box> is defined for <coord-box> (and other productions), which I forgot to define.

Here is the (simplified) representation of the data I expect to find in w3c/webref: 
  "values": [
    {
      "name": "box",
      "values": [
        {
          "name": "coord-box",
          "value": "<paint-box> | view-box",
          "values": [
            {
              "name": "paint-box",
              "value": "<visual-box> | fill-box | stroke-box",
              "values": [
                {
                  "name": "visual-box",
                  "value": "content-box | padding-box | border-box",
                  "values": [{ "name": "content-box" }, { "name": "padding-box" }, { "name": "border-box" }]
                },
                { "name": "fill-box" },
                { "name": "view-box" }
              ]
            }
          ]
        },
        {
          "name": "geometry-box",
          "value": "<layout-box> | fill-box | stroke-box | view-box",
          "values": [
            {
              "name": "layout-box",
              "value": "<visual-box> | margin-box",
              "values": [
                {
                  "name": "visual-box",
                  "value": "content-box | padding-box | border-box",
                  "values": [{ "name": "content-box" }, { "name": "padding-box" }, { "name": "border-box" }]
                },
                { "name": "margin-box" }
              ]
            },
            { "name": "fill-box" },
            { "name": "stroke-box" },
            { "name": "view-box" }
          ]
        },
        {
          "name": "layout-box",
          "value": "<visual-box> | margin-box",
          "values": [
            {
              "name": "visual-box",
              "value": "content-box | padding-box | border-box",
              "values": [{ "name": "content-box" }, { "name": "padding-box" }, { "name": "border-box" }],
            },
            { "name": "margin-box" }
          ]
        },
        {
          "name": "paint-box",
          "value": "<visual-box> | fill-box | stroke-box",
          "values": [
            {
              "name": "visual-box",
              "value": "content-box | padding-box | border-box",
              "values": [{ "name": "content-box" }, { "name": "padding-box" }, { "name": "border-box" }],
            },
            { "name": "fill-box" },
            { "name": "view-box" }
          ]
        },
        {
          "name": "visual-box",
          "value": "content-box | padding-box | border-box",
          "values": [{ "name": "content-box" }, { "name": "padding-box" }, { "name": "border-box" }],
        },
      ]
    },
  ]

Would it be ok? Cc @tidoust for a review.

Actually, I do not care about the context (namespace) of a production because I am only interested by its syntax (value), assuming I am right to consider it context-insensitive.

And I am not sure it is right to restrict eg. <visual-box> for all other <-*box> productions. One may think that <visual-box> cannot be used outside of these contexts.

I do not even need <box> references to be removed from CSS Box. I just need the changes in CSS Backgrounds 4 and CSS Shapes. So feel free to make whatever changes you think are appropriate.

@cdoublev
Copy link
Collaborator Author

cdoublev commented Nov 6, 2023

I went ahead and defined each keyword with the appropriate contexts. Bikeshed does not report any errors.

@tabatkins tabatkins merged commit 798ba91 into w3c:main Nov 10, 2023
@cdoublev cdoublev deleted the pr-9505 branch November 11, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cdoublev @w3cbot @tabatkins