Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2258793003:
Use LocalStyleChange for insertion point inheritance propagation. (Closed)
|
Created:
4 years, 4 months ago by rune Modified:
4 years, 3 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse LocalStyleChange for insertion point inheritance propagation.
For shadow dom v0, we used a SubtreeStyleChange for propagating
inherited style changes through insertion points to distributed nodes.
LocalStyleChange should suffice. We already use LocalStyleChange in the
HTMLSlotElement case.
We still need to use SubtreeStyleChange where we have a
SubtreeStyleChange/Force from further up the tree like:
<host>
<:shadow-root>
<style>.a::content * { background: green }</style>
<div id="a">
<content></content>
</div>
</:shadow-root>
<div>Green when #a gets class a.</div>
</host>
R=kochi@chromium.org
BUG=638869
Committed: https://crrev.com/dfe6cf582c76a362871d93821a25ee5af7037cc1
Cr-Commit-Position: refs/heads/master@{#414386}
Patch Set 1 #Patch Set 2 : Rebased #
Messages
Total messages: 34 (16 generated)
The CQ bit was checked by rune@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ptal
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ping
Sorry for the delay, I missed the original request - and thanks for pinging. LGTM.
The CQ bit was checked by rune@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by rune@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org Link to the patchset: https://codereview.chromium.org/2258793003/#ps20001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by rune@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by rune@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
sashab@chromium.org changed reviewers: + sashab@chromium.org
This is *awesome* rune. Btw I'd like your thoughts on the comparisons like "change > Inherit" etc... Would be better self-documenting to have utility functions that capture this, so when we update the enum its clearer where the update affects? Hard to think of function names though.. Eg "changeRequiresPropagation" (change >= independentinherit), "changeRequiresRecalcAndPropagation" (change >= inherit), etc. Also in each function we could check equality on any of the required values, not using < or >.
The CQ bit was checked by rune@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from
==========
Use LocalStyleChange for insertion point inheritance propagation.
For shadow dom v0, we used a SubtreeStyleChange for propagating
inherited style changes through insertion points to distributed nodes.
LocalStyleChange should suffice. We already use LocalStyleChange in the
HTMLSlotElement case.
We still need to use SubtreeStyleChange where we have a
SubtreeStyleChange/Force from further up the tree like:
<host>
<:shadow-root>
<style>.a::content * { background: green }</style>
<div id="a">
<content></content>
</div>
</:shadow-root>
<div>Green when #a gets class a.</div>
</host>
R=kochi@chromium.org
BUG=638869
==========
to
==========
Use LocalStyleChange for insertion point inheritance propagation.
For shadow dom v0, we used a SubtreeStyleChange for propagating
inherited style changes through insertion points to distributed nodes.
LocalStyleChange should suffice. We already use LocalStyleChange in the
HTMLSlotElement case.
We still need to use SubtreeStyleChange where we have a
SubtreeStyleChange/Force from further up the tree like:
<host>
<:shadow-root>
<style>.a::content * { background: green }</style>
<div id="a">
<content></content>
</div>
</:shadow-root>
<div>Green when #a gets class a.</div>
</host>
R=kochi@chromium.org
BUG=638869
Committed: https://crrev.com/dfe6cf582c76a362871d93821a25ee5af7037cc1
Cr-Commit-Position: refs/heads/master@{#414386}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/dfe6cf582c76a362871d93821a25ee5af7037cc1 Cr-Commit-Position: refs/heads/master@{#414386}
Message was sent while issue was closed.
On 2016/08/25 02:49:48, sashab wrote: > This is *awesome* rune. Btw I'd like your thoughts on the comparisons like > "change > Inherit" etc... Would be better self-documenting to have utility > functions that capture this, so when we update the enum its clearer where the > update affects? > > Hard to think of function names though.. Eg "changeRequiresPropagation" (change > >= independentinherit), "changeRequiresRecalcAndPropagation" (change >= > inherit), etc. Also in each function we could check equality on any of the > required values, not using < or >. I landed this since I already had an lgtm and the land was in progress. I can follow up with another CL.
Message was sent while issue was closed.
Yeah that's np, was more looking for your input and hypothetical future changes , this CL definitely lgtm :) On Thu, Aug 25, 2016 at 9:01 PM, <rune@opera.com> wrote: > On 2016/08/25 02:49:48, sashab wrote: > > This is *awesome* rune. Btw I'd like your thoughts on the comparisons > like > > "change > Inherit" etc... Would be better self-documenting to have > utility > > functions that capture this, so when we update the enum its clearer > where the > > update affects? > > > > Hard to think of function names though.. Eg "changeRequiresPropagation" > (change > > >= independentinherit), "changeRequiresRecalcAndPropagation" (change >= > > inherit), etc. Also in each function we could check equality on any of > the > > required values, not using < or >. > > I landed this since I already had an lgtm and the land was in progress. I > can > follow up with another CL. > > > https://codereview.chromium.org/2258793003/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
Yeah that's np, was more looking for your input and hypothetical future changes , this CL definitely lgtm :) On Thu, Aug 25, 2016 at 9:01 PM, <rune@opera.com> wrote: > On 2016/08/25 02:49:48, sashab wrote: > > This is *awesome* rune. Btw I'd like your thoughts on the comparisons > like > > "change > Inherit" etc... Would be better self-documenting to have > utility > > functions that capture this, so when we update the enum its clearer > where the > > update affects? > > > > Hard to think of function names though.. Eg "changeRequiresPropagation" > (change > > >= independentinherit), "changeRequiresRecalcAndPropagation" (change >= > > inherit), etc. Also in each function we could check equality on any of > the > > required values, not using < or >. > > I landed this since I already had an lgtm and the land was in progress. I > can > follow up with another CL. > > > https://codereview.chromium.org/2258793003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
