Skip to content

Serialize property display #461

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Serialize property display #461

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a48abf6
Serialize property display
jm318 Aug 22, 2017
3a7b05f
Changed serialization description of display
jm318 Aug 22, 2017
796434f
Merge branch 'master' into string
jm318 Aug 22, 2017
01d4142
Changed descriptions to address review comments'
jm318 Aug 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions css-typed-om/Overview.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ The <dfn method for=StylePropertyMap>append(DOMString <var>property</var>,

3. If {{StylePropertyMap}}’s [=property model=] contains an entry for |property|,
return the first value found in that entry.

4. Else, return `null`.
</div>

Expand Down Expand Up @@ -2275,7 +2275,7 @@ objects, the {{CSSURLImageValue/url}} attribute contains the URL that references
when called,
perform the following steps:

1. If the |url| passed into the constructor doesn't correctly
1. If the |url| passed into the constructor doesn't correctly
parse as a <<url>>, throw a {{TypeError}} and exit this algorithm.
2. Else, return a new {{CSSURLImageValue}}
with its {{CSSURLImageValue/url}} internal slot
Expand Down Expand Up @@ -2879,7 +2879,7 @@ depending on the property they came from:
3. If 'border-image-slice' does not specify ''100%'' for all sides and omits the ''border-image-slice/fill'' keyword,
serialize 'border-image-slice' and append it to |values|.
4. If 'border-image-width' does not specify ''1'' for all sides,
append "/ " (U+002F FORWARD SLASH followed by U+0020 SPACE)
append "/ " (U+002F FORWARD SLASH followed by U+0020 SPACE)
to the result of serializing 'border-image-width' and append it to |values|.
5. If 'border-image-outset' does not specify ''0'' for all sides:
1. If the previous 'border-image-width' step did not append anything to |values|,
Expand All @@ -2888,7 +2888,7 @@ depending on the property they came from:
otherwise let |prefix| be <nobr>"/ "</nobr>
(U+002F FORWARD SLASH followed by U+0020 SPACE)
2. Append |prefix|
to the result of serializing 'border-image-outset'
to the result of serializing 'border-image-outset'
and append it to |values|.
6. If 'border-image-repeat' is not ''border-image-repeat/stretch'' in both axises,
serialize 'border-image-repeat' and append it to |values|.
Expand All @@ -2903,6 +2903,44 @@ depending on the property they came from:
return "currentcolor".
2. Otherwise, return the result of serializing the <<color>> value.

: 'display'
::
1. Let |values| initially be the empty [=list=].
2. If 'display-outside' or 'display-inside' is specified:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These properties don't exist. You probably mean the <<display-inside>> and <<display-outside>> values. ^_^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for pointing out! :)

1. Append value of 'display-outside' to |values|.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value is an abstract CSS value; need to serialize it to produce a JS value. (Same applies to all the steps.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

2. Append value of 'display-inside' to |values|.
3. Goto step 7.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means you won't hit the list-item step at all if you specified something like display: block list-item.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and I understand this is what the spec says according to "|", i.e. these are mutually exclusive?

3. If 'display-listitem' is specified:
1. Append ''list-item'' to |values|.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Append a string, "list-item", not a CSS value ''list-item''.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

2. If 'display-outside' is specified, serialize it and append it to |values|.
3. If ''flow'' or ''flow-root'' is specified, append it to |values|.
4. Goto step 7.
4. If 'display-internal' is specified:
1. Append its value to |values|.
2. Goto step 7.
5. If 'display-box' is specified:
1. Append its value to |values|.
2. Goto step 7.
6. If 'display-legacy' is specified:
1. Append its value to |values|.
2. Goto step 7.
7. If |values| is:
1. ''block flow'', return 'block'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|values| is a list of strings, not a sequence of CSS values. Needs to be «"block", "flow"», per the Infra Standard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And 'block' isn't a valid property, and should be a string instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for pointing out!

2. ''block flow-root'', return ''flow-root''.
3. ''inline flow'', return ''inline''.
4. ''inline flow-root'', return ''inline-block''.
5. ''run-in flow'', return ''run-in''.
6. ''list-item block flow'', return ''list-item''.
7. ''list-item inline flow'', return ''inline list-item''.
8. ''block flex'', return ''flex''.
9. ''inline flex'', return ''inline-flex''.
10. ''block grid'', return ''grid''.
11. ''inline ruby'', return ''ruby''.
12. ''block table'', return ''table''.
13. ''inline table'', return ''inline-table''.
14. [=list/empty=], return "none".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it end up empty, and how does emptiness equate to display: none? none is handled by the <<display-box>> clause in step 5, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would be empty if property is simply "display:", but I guess it's illegal anyway. So I removed the case for empty-list.

15. otherwise, return the result of concatenating all the items in |values|,
separated by " " (U+0020 SPACE).


Security Considerations {#security-considerations}
Expand Down