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
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator Author

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
Copy Markdown
Collaborator Author

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> Oct 20, 2023
@w3cbot
Copy link
Copy Markdown

w3cbot commented Oct 20, 2023

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

@tabatkins
Copy link
Copy Markdown
Member

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
Copy Markdown
Collaborator Author

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
Copy Markdown
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

Labels

None yet

3 participants