[css-display] Implement the multi-keyword syntax for the 'display' property
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete, DevAdvocacy, Whiteboard: [DevRel:P1] [layout:backlog][wptsync upstream])
Attachments
(17 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
52 bytes,
text/x-github-pull-request
|
Details | Review |
This bug is only intended for making 'display' a shorthand, not add any new values we don't currently support. (Bug 907396 will add support for 'contents'.)
Comment 1•10 years ago
•
|
||
> This bug is only intended for making 'display' a shorthand, not add any new values we don't currently support.
Then the title is misleading. How about turning this into a meta-issue and creating new ones for turning 'display' into a shorthand and the new properties blocking this issue?
Sebastian
Updated•10 years ago
|
Updated•7 years ago
|
Comment 2•6 years ago
|
||
As asked in comment 1, make this a meta-bug and file individual ones for the different parts it covers? Sebastian
Assignee | ||
Comment 3•6 years ago
|
||
The spec has changed again since this bug was filed, re-summarizing accordingly. 'display' is now still a longhand, but it now accepts multiple-keyword values.
Comment 4•6 years ago
|
||
CSS Display Level 3 has just gone back to CR. I would like to see Mozilla implement the new "full display" values fairly quickly — the ones that are simple aliases to preexisting properties. Which is hopefully a fairly trivial amount of work. (There are two new display values, `inline list-item`, aka `inline flow list-item` and `block ruby`. Those can be handled in separate Bugzilla tickets, as they will require more work.) To experts on CSS, it might not seem like a big deal, to offer the ability to write `display: block grid` instead of `display: grid` in their code, but I'm very excited about this change to CSS. It will make teaching layout much easier. Conceptually, thinking of display-outside and display-inside as two different concepts to master — this is huge. It's an especially good time to have this change, since the industry is just now figuring out how to do layout in vanilla CSS, now that we have CSS Grid, Flexbox, et al, and people are letting go of layout frameworks like Bootstrap and Foundation. Many, many people are in the process of learning CSS for layout for the first time. I'd love to see this shipped in 2019, and not 2022. I plan to teach layout using `display: [ <display-outside> || <display-inside> ]` immediately, letting people know it doesn't work in any browser yet, but will. It'd be great if that story could change to "works in Firefox". I would also be helpful if we could write tests for the other browsers to use, speeding up adoption by others.
Comment 6•6 years ago
|
||
(In reply to Jen Simmons [:jensimmons] from comment #4) > CSS Display Level 3 has just gone back to CR. > > I would like to see Mozilla implement the new "full display" values fairly > quickly — the ones that are simple aliases to preexisting properties. Which > is hopefully a fairly trivial amount of work. For reference, the spec. has a table with a line-up of single-keyword and multi-keyword values: https://drafts.csswg.org/css-display/#display-value-summary > I plan to teach layout using `display: [ <display-outside> || > <display-inside> ]` immediately, letting people know it doesn't work in any > browser yet, but will. It'd be great if that story could change to "works in > Firefox". For what it's worth, that is also what https://developer.mozilla.org/en-US/docs/Web/CSS/display is already doing for quite some time. Sebastian
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
I have a WIP implementing this, so I might as well take it...
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D39753
Assignee | ||
Comment 10•5 years ago
|
||
Depends on D39754
Assignee | ||
Comment 11•5 years ago
|
||
Depends on D39755
Assignee | ||
Comment 12•5 years ago
|
||
Depends on D39756
Assignee | ||
Comment 13•5 years ago
|
||
Depends on D39757
Assignee | ||
Comment 14•5 years ago
|
||
Depends on D39758
Assignee | ||
Comment 15•5 years ago
|
||
Depends on D39759
Assignee | ||
Comment 16•5 years ago
|
||
Depends on D39760
Assignee | ||
Comment 17•5 years ago
|
||
Depends on D39761
Assignee | ||
Comment 18•5 years ago
|
||
Depends on D39762
Assignee | ||
Comment 19•5 years ago
|
||
Depends on D39763
Assignee | ||
Comment 20•5 years ago
|
||
Depends on D39764
Assignee | ||
Comment 21•5 years ago
|
||
Depends on D39765
Assignee | ||
Comment 22•5 years ago
|
||
Depends on D39766
Assignee | ||
Comment 23•5 years ago
|
||
Depends on D39767
Assignee | ||
Comment 24•5 years ago
|
||
FYI, most of these parts are idempotent, but I figured it'd be easier to review them in smaller steps...
Part 12/14/15 is where the meat is. Also, these patches adds no new display values per se, only new syntax aliases for existing box types. (new values will be added separately in the blocked bugs)
Assignee | ||
Comment 25•5 years ago
|
||
... also, the bugs that adds inline list-items and 'block ruby' will cleanup the parse/toCSS code a bit, FYI.
Assignee | ||
Comment 26•5 years ago
|
||
BTW, I've tried to mark all the Rust changes with the appropriate #[cfg(feature = "gecko"/"servo")]
but I haven't tried building Servo with any of these changes...
Comment 27•5 years ago
|
||
Can parts 1 through 11 land now that they’re accepted, before parts 12 through 16 get finished?
Comment 28•5 years ago
|
||
I've been assigned the dev docs task for this, as something that will ship in 69. Just wanted to check before starting that it is shipping in 69, and whether there are any parts that might not, anything I need to flag as a note, or against the BCD for the feature.
Comment 29•5 years ago
|
||
Hi Rachel: This should be landing in 70, not 69. We are not planning to uplift it to the 69 beta. I'll leave it to Mats to comment on parts that may not make it.
Assignee | ||
Comment 30•5 years ago
|
||
This and bug 1105868 and bug 1557825 are for v70. I'll land these in a bit and send an intent-to-ship.
I don't know what BCD means, but the changes follows the spec and are complete with tests. No particular notes. Please let me know if anything is unclear.
Comment 31•5 years ago
|
||
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/17ca7eaa9dba part 1 - Make nsCSSKTableEntry use uint16_t to represent the value. r=emilio https://hg.mozilla.org/integration/autoland/rev/a101b9693ad6 part 2 - Remove FCDATA_FOR_DISPLAY. r=emilio https://hg.mozilla.org/integration/autoland/rev/fd0c240fd715 part 3 - Remove UNREACHABLE_FCDATA. r=emilio https://hg.mozilla.org/integration/autoland/rev/f3c8731da1e5 part 4 - Move an assertion about display:contents to a place where it's not dead code. r=emilio https://hg.mozilla.org/integration/autoland/rev/98aff703294e part 5 - Move an assertion about '-moz-{inline-}box'. r=emilio https://hg.mozilla.org/integration/autoland/rev/6a1f7235c5a9 part 6 - Simplify FindDisplayData by removing a now unnecessary lambda. r=emilio https://hg.mozilla.org/integration/autoland/rev/f4280b135e7a part 7 - Move XUL variants of '-moz-{inline-}box' from FindXULDisplayData to FindDisplayData. r=emilio https://hg.mozilla.org/integration/autoland/rev/6bb15997aba7 part 8 - Move block box creation inside the switch together with all the other display types. r=emilio https://hg.mozilla.org/integration/autoland/rev/127c7dfc011a part 9 - Move scroll box suppression inside the switch. r=emilio https://hg.mozilla.org/integration/autoland/rev/62b047361030 part 10 - Remove dead code that handles display:none/contents which shouldn't reach FindDisplayData. r=emilio https://hg.mozilla.org/integration/autoland/rev/ec8f17f71b28 part 11 - Remove FindXULDisplayData and handle all display types in FindDisplayData instead. r=emilio https://hg.mozilla.org/integration/autoland/rev/965099bbb4a6 part 12 - Implement multi-keyword 'display' values for Gecko. r=emilio https://hg.mozilla.org/integration/autoland/rev/6a674140cb8f part 13 - Add a few accessors to query the parts of a 'display' value. r=emilio https://hg.mozilla.org/integration/autoland/rev/6c19f13a6082 part 14 - Use only the DisplayInside() value to decide which type of frame to create. r=emilio https://hg.mozilla.org/integration/autoland/rev/f919a62c86d1 part 15 - Change a few existing nsStyleDisplay accessors to use DisplayInside()/DisplayOutside(). r=emilio https://hg.mozilla.org/integration/autoland/rev/57bcf2eea109 part 16 - WPT updates for new 'display' values. r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18422 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Assignee | ||
Comment 34•5 years ago
|
||
Comment 35•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/17ca7eaa9dba
https://hg.mozilla.org/mozilla-central/rev/a101b9693ad6
https://hg.mozilla.org/mozilla-central/rev/fd0c240fd715
https://hg.mozilla.org/mozilla-central/rev/f3c8731da1e5
https://hg.mozilla.org/mozilla-central/rev/98aff703294e
https://hg.mozilla.org/mozilla-central/rev/6a1f7235c5a9
https://hg.mozilla.org/mozilla-central/rev/f4280b135e7a
https://hg.mozilla.org/mozilla-central/rev/6bb15997aba7
https://hg.mozilla.org/mozilla-central/rev/127c7dfc011a
https://hg.mozilla.org/mozilla-central/rev/62b047361030
https://hg.mozilla.org/mozilla-central/rev/ec8f17f71b28
https://hg.mozilla.org/mozilla-central/rev/965099bbb4a6
https://hg.mozilla.org/mozilla-central/rev/6a674140cb8f
https://hg.mozilla.org/mozilla-central/rev/6c19f13a6082
https://hg.mozilla.org/mozilla-central/rev/f919a62c86d1
https://hg.mozilla.org/mozilla-central/rev/57bcf2eea109
Comment 36•5 years ago
|
||
Comment 37•5 years ago
|
||
Nit: this patch stack seems to have left some files in a non-clang-format
-clean state, which means some downstream commit can end up inadvertently incorporating the reformatting cleanup automatically (and if it's even noticed, it can be hard to prevent that from happening -- see e.g. bug 1556088 for some of the trouble that this can cause).
In this case, https://hg.mozilla.org/integration/autoland/rev/4d8eb840fc2eaa37f6e75e5a5f2e8863beeccf19 ended up auto-reformatting to clean up some nsCSSFrameConstructor.cpp formatting from this patch-stack here. That happened when I imported/rebased a patch, without me noticing, as part of a local commit hook set up by our mercurial extensions/tools. This is partly my fault for not noticing the reformatting leaking in there, but it's also a good reminder to keep an eye out for reviewbot notes like https://phabricator.services.mozilla.com/D39758#1263097 and to apply this reformatting up-front (or soon after the fact) so that latent reformatting changes don't automatically leak into later patches by accident.
Comment 38•5 years ago
•
|
||
BTW, I ended up backing out the inadvertently-reformatting-laden commit referenced in comment 37, and I flushed all latent reformatting in layout in bug 1574310. So everything should be good now (at least in the layout directory).
Updated•5 years ago
|
Description
•