[cssom] Serialization of a declaration block not idempotent in presence of logical properties. #3244

emilio · 2018-10-24T01:06:11Z

Testcase is:

<!doctype html>
<div id="test" style="margin-top: 10px; margin-block-start: 20px; margin-inline-start: 30px; margin-bottom: 40px; margin-left: 50px; margin-right: 60px"></div>
<pre>
<script>
  document.writeln(test.style.cssText);
</script>
</pre>

Which serializes to:

margin: 10px 60px 40px 50px; margin-block-start: 20px; margin-inline-start: 30px;

Which is not idempotent.

cc @FremyCompany

The text was updated successfully, but these errors were encountered:

emilio · 2018-10-24T01:07:05Z

cc @Loirooriol too :)

emilio · 2018-10-24T01:08:05Z

I think this is easy to fix, and we may want to just not compress shorthands if we'd need to jump over a declaration with the same logical group in order to do that.

Loirooriol · 2018-10-24T06:54:53Z

To avoid overlap with #1282 (margin may end up resetting the logical longhands), we can consider this testcase:

$0.style.cssText = "margin-block-start: 10px; margin-bottom: 20px; margin-block-end: 30px";
$0.style.cssText;

It shouldn't serialize as margin-block: 10px 30px; margin-bottom: 20px.

I agree with your solution.

Loirooriol · 2018-10-25T08:25:36Z

I think the current use of the concept in setProperty allows 3 different border logical groups (border-width, border-style and border-color), but

$0.style.cssText = "border: 1px solid red; border-block-start-color: green; border-color: blue";
$0.style.cssText;

should not be "border: 1px solid blue; border-block-start-color: green;"

So either all border longhands should be in the same logical group (I would like some clarification in #3033), or instead of checking what I initially imagined, i.e.

there is no triplet of properties A, B, C such that
- index(A) < index(B) < index(C)
- A, B, C belong to the same logical group, but B has different mapping logic than A and C
- A and C belong to current longhands

it should be

there is no pair of properties A, B such that
- index(property) < index(A) < index(B)
- A, B belong to the same logical group but have different mapping logic
- B belongs to current longhands

where property and current longhands are the ones from the algorithm in https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block

emilio · 2018-10-26T02:21:52Z

There's also the case of the property containing both the logical and the physical longhands (i.e., all)... That one's fun, since it's not defined in which order we expand the properties even... I guess since all only allows a CSS wide keyword, this doesn't really matter in practice, but...

cdoublev · 2022-10-19T14:36:52Z

This has been fixed by dc37a3f, isn't it?

Loirooriol · 2022-10-19T15:35:32Z

Firefox handles margin-block-start: 10px; margin-bottom: 20px; margin-block-end: 30px well now.
But border: 1px solid red; border-block-start-color: green; border-color: blue is still wrong. Either Firefox didn't implement the spec well, or the spec needs more refinement.

Edit: the spec seems fine

cdoublev · 2022-10-19T18:50:47Z

I have not implemented the step added by the commit I linked above. Could you please tell me why margin-block-start: 10px; margin-bottom: 20px; margin-block-end: 30px does not serialize to margin-bottom: 20px; margin-block: 10px 30px?

EDIT: hmm, I guess a round-trip would change the declaration order (margin-block-start would move after margin-bottom), therefore it should stay as specified in the initial input.

I would expect the second example to serialize to border-width: 1px; border-style: solid; border-block-start-color: green; border-color: blue? Is it correct?

Loirooriol · 2022-10-19T22:11:24Z

Don't forget border-image-*. I think it should be:

In some undefined order:
- border-style: solid
- border-width: 1px
- border-image: none 100% / 1 / 0 stretch (or equivalent)
border-block-start-color: green
border-color: blue

cdoublev · 2022-10-23T07:31:35Z

For the following case:

style.borderTopWidth = '1px'
style.borderBlockStartWidth = '2px'
style.borderTopStyle = 'solid'
style.borderTopColor = 'green'

I would expect style.cssText to be border-top: 1px solid green; border-block-start-width: 2px; but the current definition of step 6 of serialize a CSS declaration block seems to prevent serializing with border-top.

If so, I think it could be modified with:

- If there’s any declaration in `declaration block` in between the first and the last longhand in `current longhands` which belongs to the same logical property group, but has a different mapping logic as any of the longhands in `current longhands`, and is not in `current longhands`, continue with the steps labeled `shorthand loop`.
+ If there’s any declaration in `declaration block` in between some declarations in `current longhands` which belongs to the same logical property group, but has a different mapping logic as any of the longhands of these declarations, and is not in `current longhands`, continue with the steps labeled `shorthand loop`.

Loirooriol · 2022-10-23T20:29:05Z

I think the spec is saying the same as your wording, so it shouldn't be preventing the serialization with border-top.
Basically, being between some longhands that match a certain condition is equivalent to being between the first and the last longhands that match the condition.

But I agree the spec is not particularly clear, I had to read it a few times to understand it. Your wording is not particularly clear to me either, what does "these declarations" refer to? This looks better to me:

If there’s any declaration in declaration block in between the first and the last longhand in current longhands which is not in current longhands, and belongs to the same logical property group but has a different mapping logic as any of the longhands in current longhands, continue with the steps labeled shorthand loop.

Note that now it's clear that the "which" refers to "any declaration" and not to "first/last longhand in current longhands".
And no comma after "same logical property group", otherwise it's not clear that "as any of the longhands in current longhands" refers to both "same" and "different".

cdoublev · 2022-10-24T12:09:41Z

Basically, being between some longhands that match a certain condition is equivalent to being between the first and the last longhands that match the condition.

In my example, the declaration for border-block-start-width is between the first and last declaration of current longhands, but it is not between declarations whose longhands are in the same logical property group. There should be three declarations whose longhands are in the same logical property group: the declaration not in current longhands, and the "wrapping" declarations in current longhands.

To make it clear:

style.borderTopWidth = '1px'
style.borderBlockStartWidth = '2px'
// Some border-*-width is missing here 
// so that the above declaration is in between two declarations 
// whose longhands are in the same logical property group
style.borderTopStyle = 'solid'
style.borderTopColor = 'green'
style.cssText; // border-top: 1px solid green; border-block-start-width: 2px;

style.borderTopWidth = '1px'
style.borderBlockStartWidth = '2px'
style.borderRightWidth = '1px'
style.borderBottomWidth = '1px'
style.borderLeftWidth = '1px'
style.cssText; // border-top-width: 1px; border-block-start-width: 2px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px;

Note that Firefox serializes style.cssText to border-top-width: 1px; border-block-start-width: 2px; border-top-style: solid; border-top-color: green; in the first example. It may be right because the order of declarations would be preserved with a round-trip. I do not know if this should be a requirement for shorthand serialization.

I agree with your rewording. I also considered splitting the sentence (step) by listing conditions but ended up only rewording it to make my point. I was also not comfortable with a declaration in between longhands in current longhands because a declaration is compared to longhands, and current longhands contains declarations, not longhands.

But I am sure someone can come up with the ideal and correct wording.

Loirooriol · 2022-10-24T15:17:42Z

but it is not between declarations whose longhands are in the same logical property group

Precisely, the spec says "belongs to the same logical property group, but has a different mapping logic as any of the longhands in current longhands". So border-block-start-width doesn't have to be between 2 longhands in the same property group, just having border-top-width is enough.

Loirooriol · 2022-10-24T15:45:51Z

OK, sorry, I'm contradicting myself, that's what happens when I try to post quickly. So yeah, I guess that the spec is preventing the serialization with border-top even though it would be fine.

But note that requiring it to be between 2 declaration in the same logical group would be wrong, see #3244 (comment)

cdoublev · 2022-10-24T18:47:37Z

But note that requiring it to be between 2 declaration in the same logical group would be wrong, see #3244 (comment)

border should be skipped because the declaration for border-block-start-color is stored before other border-*-color declarations, right?

I guess it only applies to shorthands (there is only border) whose longhands belong to more than one logicial property group. In this case, the current wording is fine, yes.

Loirooriol · 2022-10-25T14:18:35Z

border should be skipped because the declaration for border-block-start-color is stored before other border-*-color declarations, right?

Exactly, we can't serialize border before border-block-start-color since some physical longhands of border in the border-color-* logical property group appear after border-block-start-color.

I guess it only applies to shorthands (there is only border) whose longhands belong to more than one logical property group

Not necessarily more than one logical property group. It suffices to have some longhands in one group (without covering the group completely), and some longhands not in the group (either another group or no group). But yeah I think border is the only case for now.

The spec could say:

If there’s any declaration in declaration block such that:

It's not in current longhands

It appears after the first longhand in current longhands

It appears before some longhand in current longhands that belongs to the same logical property group but has a different mapping logic

cdoublev · 2022-10-26T07:43:02Z

I wonder when a declaration that would not be in current longhand and belong to the same logical property group than some longhands in current longhand, would not have a different mapping logic. This condition can be removed, isn't it?

I guess it could be further improved by serializing the interleaved declaration, in order to serialize border: 1px solid red; border-block-start-color: green; border-color: blue to border-block-start-color: green; border: 1px solid blue;. But this would prevent serializing eg. border: 1px solid red; border-block-start-color: green; border-block-end-color: green; border-color: blue to border-block-color: green; border: 1px solid blue;... unless you look for all interleaved declarations: fun stuff!

Another example for the observation I made in the previous paragraph is the output for style.cssText after declaring all to initial and removing/updating any longhand declaration: since physical longhands must now be canonically ordered after the corresponding logical longhands, border cannot be used because its reset-only sub-properties are declared before the logical border-* properties, as well as corners because corner-shape is declared before and border-radius-* are declared after the logical border-radius-* properties. Ordering all border and corners sub-properties after the logical properties would be a workaround for these specific cases though.

cdoublev · 2022-10-31T04:44:45Z

I managed to implement the improvement mentioned in my previous comment. Basically it boils down to (if some condition is satisfied) resuming Declaration loop with the interfering declaration from step 6 (the one that is not in current longhands) before the current declaration (the first declaration of current longhands).

6. If there’s any declaration interfering in declaration block such that... :

It's not in current longhands

It appears after the first longhand in current longhands

It appears before some declaration in current longhands whose name belongs to the same logical property group but has a different mapping logic than the name of interfering

... then if all declarations in current longhands appearing before interfering in declaration block, are not in the same logical property group or have the same mapping logic than the name of interfering, resume the steps labeled declaration loop with interfering, otherwise resume the steps labeled shorthand loop.

Similarly as the obvervation I made in a previous comment, I think either are not in the same logical property group or have the same mapping logic is required.

Test cases to recap:

(Below, move declarations would be the result from a round trip)

// Currently resolved: move declarations backward
style.cssText = 'border-top-width: 1px; border-block-start-width: 1px; border-top-style: solid; border-top-color: green'
expect(style.cssText).toBe('border-top: 1px solid green; border-block-start-width: 1px;')

// Improvement: move declarations forward
style.cssText = 'border-top-width: 1px; border-block-start-style: none; border-top-style: solid; border-top-color: green'
expect(style.cssText).toBe('border-block-start-style: none; border-top: 1px solid green;')

// Guarded: skip shorthand when declarations cannot be moved backward/forward
let input = 'border-top-width: 1px; border-block-start-width: 1px; border-block-start-style: none; border-top-style: solid; border-top-color: green'
expect(style.cssText).toBe(input)

// More complex cases with moving declarations forward
style.cssText = 'border: 1px solid red; border-block-start-color: orange; border-color: green'
expect(style.cssText).toBe('border-block-start-color: orange; border: 1px solid green;')
style.cssText = 'border-top-width: 1px; border-block-start-width: 1px; border-block-end-width: 1px; border-top-style: solid; border-top-color: green'
expect(style.cssText).toBe('border-top: 1px solid green; border-block-width: 1px;')
style.cssText = 'border-block-width: 1px; border-top-width: 2px; border-top-style: none; border-block-style: solid; border-block-color: green;'
expect(style.cssText).toBe('border-top-style: none; border-block: 1px solid green; border-top-width: 2px;')

Let me know what you think.

Loirooriol · 2022-10-31T18:12:43Z

resume the steps labeled declaration loop with interfering

Can't just jump from declaration to interfering, since there could be unrelated declarations in between:

border-top-width: 1px; color: red; border-block-start-style: none; border-top-style: solid; border-top-color: green

Should not skip color: red. Also, there is no guarantee that shorthand will be serializable by the time that we reach the declaration in current longhands that appears after interfering and has the same logical group with opposite logic.

My concern with trying too hard to find the best serialization when there are logical properties is that it can make it much harder to also improve things like #2515. Optimizing for 2 different things is tricky.

cdoublev · 2022-10-31T19:48:21Z

Can't just jump from declaration to interfering, since there could be unrelated declarations in between

Sorry, I did not mean to copy/paste code, but I meant declarations.splice(declarations.indexOf(currentLonghands[0]), 0, ...declarations.splice(declarations.indexOf(interfering), 1)) in JS code (were declarations is a shallow copy of the block declarations).

You also "jump" declarations when you look forward, no?

My concern with trying too hard to find the best serialization when there are logical properties is that it can make it much harder to also improve things like #2515.

Yeah, I know about this issue but I have not given much thought to it. The problem with find the best serialization seems to draw the line where it becomes too much when crossed.

Loirooriol · 2022-11-02T05:16:13Z

Well then the iteration order needs to be precisely defined. Note that when handling interfering then it may be potentially serializable via some shorthand, which may have its own interfering, and so on. Also note that serializing interfering first seems a bad idea if shorthand ends up not being serializable.

cdoublev · 2022-11-02T06:31:32Z

Damn, you are right. It fails with the following entries (assuming the order is guaranteed).

    const input = {
        'border-block-start-width': '1px',
        'border-top-style': 'none',
        'border-block-end-style': 'solid',
        'border-right-style': 'none',
        'border-bottom-style': 'none',
        'border-left-style': 'none',
        'border-block-start-style': 'solid',
        'border-block-start-color': 'green',
    }
    const expected = {
        'border-top-style': 'none',
        'border-block-end-style': 'solid',
        'border-right-style': 'none',
        'border-bottom-style': 'none',
        'border-left-style': 'none',
        'border-block-start': '1px solid green',
    }

I revert to emilio's algorithm (which fixes this issue), ie. abort searching a shorthand when encoutering the first condition that might cause the serialization to not be idempotent, and I think this issue can be closed.

Thanks for the follow up!

emilio added the cssom-1 label Oct 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cssom] Serialization of a declaration block not idempotent in presence of logical properties. #3244

[cssom] Serialization of a declaration block not idempotent in presence of logical properties. #3244

emilio commented Oct 24, 2018

emilio commented Oct 24, 2018

emilio commented Oct 24, 2018

Loirooriol commented Oct 24, 2018 •

edited

Loading

Loirooriol commented Oct 25, 2018 •

edited

Loading

emilio commented Oct 26, 2018

cdoublev commented Oct 19, 2022

Loirooriol commented Oct 19, 2022 •

edited

Loading

cdoublev commented Oct 19, 2022 •

edited

Loading

Loirooriol commented Oct 19, 2022

cdoublev commented Oct 23, 2022

Loirooriol commented Oct 23, 2022

cdoublev commented Oct 24, 2022 •

edited

Loading

Loirooriol commented Oct 24, 2022

Loirooriol commented Oct 24, 2022

cdoublev commented Oct 24, 2022

Loirooriol commented Oct 25, 2022

cdoublev commented Oct 26, 2022 •

edited

Loading

cdoublev commented Oct 31, 2022 •

edited

Loading

Loirooriol commented Oct 31, 2022

cdoublev commented Oct 31, 2022

Loirooriol commented Nov 2, 2022

cdoublev commented Nov 2, 2022

[cssom] Serialization of a declaration block not idempotent in presence of logical properties. #3244

[cssom] Serialization of a declaration block not idempotent in presence of logical properties. #3244

Comments

emilio commented Oct 24, 2018

emilio commented Oct 24, 2018

emilio commented Oct 24, 2018

Loirooriol commented Oct 24, 2018 • edited Loading

Loirooriol commented Oct 25, 2018 • edited Loading

emilio commented Oct 26, 2018

cdoublev commented Oct 19, 2022

Loirooriol commented Oct 19, 2022 • edited Loading

cdoublev commented Oct 19, 2022 • edited Loading

Loirooriol commented Oct 19, 2022

cdoublev commented Oct 23, 2022

Loirooriol commented Oct 23, 2022

cdoublev commented Oct 24, 2022 • edited Loading

Loirooriol commented Oct 24, 2022

Loirooriol commented Oct 24, 2022

cdoublev commented Oct 24, 2022

Loirooriol commented Oct 25, 2022

cdoublev commented Oct 26, 2022 • edited Loading

cdoublev commented Oct 31, 2022 • edited Loading

Loirooriol commented Oct 31, 2022

cdoublev commented Oct 31, 2022

Loirooriol commented Nov 2, 2022

cdoublev commented Nov 2, 2022

Loirooriol commented Oct 24, 2018 •

edited

Loading

Loirooriol commented Oct 25, 2018 •

edited

Loading

Loirooriol commented Oct 19, 2022 •

edited

Loading

cdoublev commented Oct 19, 2022 •

edited

Loading

cdoublev commented Oct 24, 2022 •

edited

Loading

cdoublev commented Oct 26, 2022 •

edited

Loading

cdoublev commented Oct 31, 2022 •

edited

Loading