Closed
Bug 1227766
Opened 9 years ago
Closed 9 years ago
will-change should sometimes cause creation of containing block for fixed-positioned elements
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
Attachments
(4 files, 3 obsolete files)
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
https://drafts.csswg.org/css-will-change/#valdef-will-change-custom-ident now says:
If any non-initial value of a property would cause the element to generate a containing block for fixed-position elements, specifying that property in will-change must cause the element to generate a containing block for fixed-position elements.
I don't think it said this back when we initially implemented will-change.
We should implement this.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → dbaron
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8691685 -
Flags: review?(dholbert)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8691686 -
Flags: review?(dholbert)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8691687 -
Flags: review?(dholbert)
Assignee | ||
Comment 4•9 years ago
|
||
Without the previous patches, the tests:
will-change-fixpos-cb-contain-1.html
will-change-fixpos-cb-filter-1.html
will-change-fixpos-cb-perspective-1.html
will-change-fixpos-cb-transform-style-1.html
fail, but they pass with the patches. The new tests:
will-change-fixpos-cb-height-1.html
will-change-fixpos-cb-transform-1.html
pass both ways: the first because it tests that nothing happens, and
the second because we were separately testing the will-change bit for
transform via HasTransform.
Attachment #8691688 -
Flags: review?(dholbert)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8691690 -
Flags: review?(dholbert)
Assignee | ||
Updated•9 years ago
|
Attachment #8691687 -
Attachment is obsolete: true
Attachment #8691687 -
Flags: review?(dholbert)
Assignee | ||
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
Comment on attachment 8691685 [details] [diff] [review]
patch 1 - Add flag for CSS properties that establish a containing block for fixed positioned elements
Review of attachment 8691685 [details] [diff] [review]:
-----------------------------------------------------------------
r=me on part 1 (and sorry for the delay -- was busy/AFK over Thanksgiving holiday long-weekend). Just one note:
::: layout/style/nsCSSProps.h
@@ +256,5 @@
> #define CSS_PROPERTY_INTERNAL (1<<28)
>
> +// This property has values that can establish a containing block for
> +// fixed positioned and absolutely positioned elements.
> +#define CSS_PROPERTY_FIXPOS_CB (1<<29)
Might be worth referencing nsStyleDisplay::IsFixedPosContainingBlock() from this comment, and/or updating that function's documentation to mention this CSS_PROPERTY_FIXPOS_CB flag. (Presumably that function & the list of properties with this flag need to be kept in sync.)
Attachment #8691685 -
Flags: review?(dholbert) → review+
Comment 8•9 years ago
|
||
(/me notices that part 4 does actually update IsFixedPosContainingBlock. My suggestion in comment 7 is still applicable, though -- the preexisting pieces of IsFixedPosContainingBlock() do still need to match the list of properties for which CSS_PROPERTY_FIXPOS_CB is set, I think, and that's worth documenting somewhere.)
Comment 9•9 years ago
|
||
Comment on attachment 8691686 [details] [diff] [review]
patch 2 - Add will-change bit for establishing a containing block for fixed positioned elements
Review of attachment 8691686 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8691686 -
Flags: review?(dholbert) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8691688 [details] [diff] [review]
patch 4 - Tests for will-change establishing a fixed-pos and abs-pos containing block
Review of attachment 8691688 [details] [diff] [review]:
-----------------------------------------------------------------
r=me on the tests, just one copypaste bug to address:
::: layout/reftests/w3c-css/submitted/will-change/will-change-fixpos-cb-contain-1.html
@@ +1,3 @@
> +<!DOCTYPE html>
> +<meta charset="utf-8">
> +<title>CSS will-change: 'will-change: filter' creates a stacking context</title>
This title is incorrect:
(1) We're really testing "contain", not "filter"
(2) We're really testing that it creates a fixed-position containing block (not that it creates a stacking context)
Looks like all of the tests here have this same <title> -- needs updating in all of them (with a different property in each test).
::: layout/reftests/w3c-css/submitted/will-change/will-change-fixpos-cb-height-1.html
@@ +1,3 @@
> +<!DOCTYPE html>
> +<meta charset="utf-8">
> +<title>CSS will-change: 'will-change: filter' creates a stacking context</title>
(When fixing this copypaste <title> issue mentioned above, note that this particular test's title needs a different tweak from the rest -- here, we're testing that we do *not* create a fixedpos containing block. Whereas the rest of the tests are testing that we *do* create a fixedpos containing block.)
Attachment #8691688 -
Flags: review?(dholbert) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8691690 [details] [diff] [review]
patch 3 - Make will-change cause creation of a containing block for fixed and absolutely positioned elements when needed
Review of attachment 8691690 [details] [diff] [review]:
-----------------------------------------------------------------
r=me on part 3
Attachment #8691690 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/02f0d955ce78b140efcbf42e8027cd3d7a92cdc5
Bug 1227766 patch 1 - Add flag for CSS properties that establish a containing block for fixed positioned elements. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/d272cd8346700eca2940496711697bada98f01f8
Bug 1227766 patch 2 - Add will-change bit for establishing a containing block for fixed positioned elements. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/daf35b346b5d103dce479ac8368a642d867cacd7
Bug 1227766 patch 3 - Make will-change cause creation of a containing block for fixed and absolutely positioned elements when needed. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ed1feeef7400a8a6d9fd1dd406d552294aa164
Bug 1227766 patch 4 - Tests for will-change establishing a fixed-pos and abs-pos containing block. r=dholbert
Comment 13•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/02f0d955ce78
https://hg.mozilla.org/mozilla-central/rev/d272cd834670
https://hg.mozilla.org/mozilla-central/rev/daf35b346b5d
https://hg.mozilla.org/mozilla-central/rev/e7ed1feeef74
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment hidden (typo) |
Comment hidden (typo) |
Comment hidden (typo) |
Assignee | ||
Updated•9 years ago
|
Attachment #8696207 -
Attachment is obsolete: true
Attachment #8696207 -
Flags: review?(dholbert)
Comment hidden (typo) |
Comment hidden (typo) |
Comment hidden (typo) |
Assignee | ||
Updated•9 years ago
|
Attachment #8696207 -
Attachment is obsolete: false
Comment hidden (typo) |
Comment hidden (typo) |
Assignee | ||
Updated•9 years ago
|
Attachment #8696207 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8696208 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•