Closed
Bug 1345362
Opened 8 years ago
Closed 8 years ago
stylo: Make -moz-pagebreak anonymous boxes non-inheriting, so their style doesn't depend on their parent
Categories
(Core :: CSS Parsing and Computation, enhancement, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(6 files)
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
heycam
:
review+
|
Details |
No description provided.
![]() |
Assignee | |
Updated•8 years ago
|
Assignee: nobody → bzbarsky
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8844800 [details] Bug 1345362 part 2. Introduce nsCSSAnonBoxes::NonInheritingTypeForPseudoTag for looking up the type of non-inheriting anonymous box for a given pseudo tag. https://reviewboard.mozilla.org/r/118112/#review119978 ::: layout/style/nsCSSAnonBoxes.h:59 (Diff revision 1) > #define CSS_ANON_BOX(_name, _value) /* nothing */ > #define CSS_NON_INHERITING_ANON_BOX(_name, _value) _name == aPseudo || > #include "nsCSSAnonBoxList.h" > #undef CSS_NON_INHERITING_ANON_BOX > #undef CSS_ANON_BOX > 0; FWIW this should probably be "false".
Attachment #8844800 -
Flags: review?(cam) → review+
Comment 6•8 years ago
|
||
Will look at the rest of these properly tomorrow, but a few things first: 1. Why the different behaviour for ResolveStyleForPlaceholder and ResolveStyleForNonInheritingAnonymousBox? 2. mNonInheritingStyleContexts is a perfect candidate for mozilla::EnumeratedArray. That would eliminate a bunch of the casts to NonInheritedBase. 3. I kind of want CSSPseudoElementType::AnonBox to be called InheritingNonBox, for symmetry. AnonBox by itself sounds like it encompass both, especially given the naming of nsCSSAnonBoxes::IsAnonBox. 4. Sorry about the StyleSetHandle setup, which results in a bit of duplication; I really should get around to turning that into a proper base class / derived class relationship.
![]() |
Assignee | |
Comment 7•8 years ago
|
||
> FWIW this should probably be "false". Agreed, will fix. > 1. Why the different behaviour for ResolveStyleForPlaceholder and ResolveStyleForNonInheritingAnonymousBox? The former never has any CSS rules applying to it. The latter can. So the former can skip all the rulewalker stuff and just pass the root rulenode when creating the stylecontext, while the latter has to figure out the right rulenode. > 2. mNonInheritingStyleContexts is a perfect candidate for mozilla::EnumeratedArray. Ah, I didn't know we had this thing! I will change it accordingly, in a changeset I will slide under these. It will definitely make things more pleasant! > 3. I kind of want CSSPseudoElementType::AnonBox to be called InheritingNonBox, for symmetry. Will do. > 4. Sorry about the StyleSetHandle setup, which results in a bit of duplication; Yeah, I tried putting this array in the Ptr when I was working on bug 1343078, but then realized that assigning a StyleSetHandle copies the Ptr, so putting something like this there doesn't work.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8845140 [details] Bug 1345362 part 1. Use mozilla::EnumeratedArray for mNonInheritingStyleContexts. https://reviewboard.mozilla.org/r/118358/#review120274
Attachment #8845140 -
Flags: review?(cam) → review+
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8844801 [details] Bug 1345362 part 3. Introduce CSSPseudoElementType::NonInheritingAnonBox to represent non-inheriting anon boxes. https://reviewboard.mozilla.org/r/118114/#review120280 ::: layout/base/GeckoRestyleManager.cpp:2876 (Diff revision 2) > styleSet->ResolveStyleWithReplacement(element, pseudoElement, > newContext, oldExtraContext, > nsRestyleHint(0)); > } > - } else if (extraPseudoType == CSSPseudoElementType::AnonBox) { > + } else if (extraPseudoType == CSSPseudoElementType::InheritingAnonBox) { > newExtraContext = styleSet->ResolveAnonymousBoxStyle(extraPseudoTag, Would you mind renaming ResolveAnonymousBoxStyle to ResolveInheritingAnonymousBoxStyle, too?
Attachment #8844801 -
Flags: review?(cam) → review+
Comment 15•8 years ago
|
||
mozreview-review |
Comment on attachment 8844802 [details] Bug 1345362 part 5. Assert that we don't have a non-inheriting anon box in nsFrame::UpdateStyleOfChildAnonBox. https://reviewboard.mozilla.org/r/118116/#review120286
Attachment #8844802 -
Flags: review?(cam) → review+
Comment 16•8 years ago
|
||
mozreview-review |
Comment on attachment 8844803 [details] Bug 1345362 part 6. Make ::-moz-pagebreak a non-inheriting anon box. https://reviewboard.mozilla.org/r/118118/#review120290
Attachment #8844803 -
Flags: review?(cam) → review+
![]() |
Assignee | |
Comment 17•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8844801 [details] Bug 1345362 part 3. Introduce CSSPseudoElementType::NonInheritingAnonBox to represent non-inheriting anon boxes. https://reviewboard.mozilla.org/r/118114/#review120280 > Would you mind renaming ResolveAnonymousBoxStyle to ResolveInheritingAnonymousBoxStyle, too? Done, added as a part 4.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 24•8 years ago
|
||
mozreview-review |
Comment on attachment 8845237 [details] Bug 1345362 part 4. Rename ResolveAnonymousBoxStyle to ResolveInheritingAnonymousBoxStyle. https://reviewboard.mozilla.org/r/118422/#review120318
Attachment #8845237 -
Flags: review?(cam) → review+
Comment 25•8 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/382046a719ff part 1. Use mozilla::EnumeratedArray for mNonInheritingStyleContexts. r=heycam https://hg.mozilla.org/integration/autoland/rev/df570f682fea part 2. Introduce nsCSSAnonBoxes::NonInheritingTypeForPseudoTag for looking up the type of non-inheriting anonymous box for a given pseudo tag. r=heycam https://hg.mozilla.org/integration/autoland/rev/8fc5d55b1e9d part 3. Introduce CSSPseudoElementType::NonInheritingAnonBox to represent non-inheriting anon boxes. r=heycam https://hg.mozilla.org/integration/autoland/rev/369ab8db0369 part 4. Rename ResolveAnonymousBoxStyle to ResolveInheritingAnonymousBoxStyle. r=heycam https://hg.mozilla.org/integration/autoland/rev/97269aca06bf part 5. Assert that we don't have a non-inheriting anon box in nsFrame::UpdateStyleOfChildAnonBox. r=heycam https://hg.mozilla.org/integration/autoland/rev/7ac30073bcac part 6. Make ::-moz-pagebreak a non-inheriting anon box. r=heycam
Updated•8 years ago
|
Priority: -- → P1
Summary: Make -moz-pagebreak anonymous boxes non-inheriting, so their style doesn't depend on their parent → stylo: Make -moz-pagebreak anonymous boxes non-inheriting, so their style doesn't depend on their parent
Comment 26•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/382046a719ff https://hg.mozilla.org/mozilla-central/rev/df570f682fea https://hg.mozilla.org/mozilla-central/rev/8fc5d55b1e9d https://hg.mozilla.org/mozilla-central/rev/369ab8db0369 https://hg.mozilla.org/mozilla-central/rev/97269aca06bf https://hg.mozilla.org/mozilla-central/rev/7ac30073bcac
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•