Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1270313002:
Handle closed mode shadow for /deep/ and ::shadow selectors (Closed)
|
Created:
5 years, 4 months ago by kochi Modified:
5 years, 4 months ago Reviewers:
hayato CC:
blink-reviews, blink-reviews-style_chromium.org, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, darktears, rwlbuis Base URL:
svn://svn.chromium.org/blink/trunk Target Ref:
refs/remotes/origin/master Project:
blink Visibility:
Public. |
DescriptionHandle closed mode shadow for /deep/ and ::shadow selectors
For closed shadow roots, /deep/ combinator or
::shadow pseudo element should not pierce the
closed shadow root boundary.
BUG=459136
TEST=closed-shadow-and-deep-combinator.html, closed-mode-deep-combinators.html
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200909
Patch Set 1 : . #
Total comments: 2
Patch Set 2 : . #Patch Set 3 : . #
Total comments: 16
Patch Set 4 : fix for comment21 #Patch Set 5 : add a layout test for 2 /deep/s #
Total comments: 8
Patch Set 6 : fix for comment29 #
Total comments: 6
Patch Set 7 : fix for comment35 #Patch Set 8 : Rename function as suggested for land #
Messages
Total messages: 46 (19 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/60001
kochi@chromium.org changed reviewers: + hayato@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I think this CL is not correct. https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:27: <div id="div1"> Each tree should have more than one nested div elements if you are using "div /deep/ div" as a test. I'm afraid that the test coverage is not enough. e.g. "div /deep/ div" defined in a closed shadow tree should be applied to the second div. <shadow-root mode='closed'> <style>div /deep/ div { ...}</style> <div><div>The rule should be applied here</div></div> </shadow-root>
Patchset #2 (id:80001) has been deleted
On 2015/08/06 02:26:52, hayato wrote: > I think this CL is not correct. > > https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... > File > LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html > (right): > > https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... > LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:27: > <div id="div1"> > Each tree should have more than one nested div elements if you are using "div > /deep/ div" as a test. > > I'm afraid that the test coverage is not enough. > > e.g. > > "div /deep/ div" defined in a closed shadow tree should be applied to the > second div. > > <shadow-root mode='closed'> > <style>div /deep/ div { ...}</style> > <div><div>The rule should be applied here</div></div> > </shadow-root> Agreed, I'll update the test case and work on fixing the code.
Patchset #2 (id:100001) has been deleted
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:140001) has been deleted
Sorry for the delay to update the CL. Could you take another look?
PTAL https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/60001/LayoutTests/fast/dom/sh... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:27: <div id="div1"> On 2015/08/06 02:26:52, hayato wrote: > Each tree should have more than one nested div elements if you are using "div > /deep/ div" as a test. > > I'm afraid that the test coverage is not enough. > > e.g. > > "div /deep/ div" defined in a closed shadow tree should be applied to the > second div. > > <shadow-root mode='closed'> > <style>div /deep/ div { ...}</style> > <div><div>The rule should be applied here</div></div> > </shadow-root> > Done.
https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:1: <!doctype html> Could you add a test for a selector which has two /deep/ or a combination of /deep/ and '::shadow'? Does it work correctly in open/closed shadow trees? https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:16: createDOM('content')), It looks div3 has more than one responsibilities in terms of tests: 1. A test for a distributed node 2. A test for nested div elements in a shadow tree, as a child of div1. It would be better to have two div elements for each purpose to isolate the responsibilities. https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:28: styleElement.offsetTop; // trigger style recalc Do we need this? Doesn't getComputedStyle() trigger style recalc? https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:30: backgroundColorShouldBe('host_open_open/div1', 'rgb(0, 0, 255)'); Could you add tests for top-level shadow hosts? e.g. backgroundColorShouldBe('host_open_open', 'rgb(0, 0, 0)'); https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:72: backgroundColorShouldBe('host_open_open/div1', 'rgba(0, 0, 0, 0)'); Instead of adding style elements altogether, could you test separately? We couldn't isolate the effects of each style element if we add them at the same time. https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:426: for (Element* element = context.element; element; element = element->parentOrShadowHostElement()) { Do we need to update this place also? FYI. https://codereview.chromium.org/1143003008/ https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... File Source/core/css/resolver/SharedStyleFinder.cpp (right): https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... Source/core/css/resolver/SharedStyleFinder.cpp:269: ShadowRoot* root1 = element().containingShadowRoot(); Could you explain why we need this check briefly?
PTAL https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:1: <!doctype html> On 2015/08/19 05:45:24, hayato wrote: > Could you add a test for a selector which has two /deep/ or a combination of > /deep/ and '::shadow'? Does it work correctly in open/closed shadow trees? A test for two /deep/s would make sense, and will add it in another CL. However, I don't think a combination of /deep/ and ::shadow needs another test, if each are tested against all open/closed combination, it should be enough. Do you come up with any special case that /deep/ + ::shadow combination? As far as I grepped, I don't see any layout test that uses both in one selector. https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:16: createDOM('content')), On 2015/08/19 05:45:24, hayato wrote: > It looks div3 has more than one responsibilities in terms of tests: > > 1. A test for a distributed node > 2. A test for nested div elements in a shadow tree, as a child of div1. > > It would be better to have two div elements for each purpose to isolate the > responsibilities. The <content> was an artifact while I was trying to add tests with ::content, which I deferred. Removed <content> from the tree. https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:28: styleElement.offsetTop; // trigger style recalc On 2015/08/19 05:45:24, hayato wrote: > Do we need this? Doesn't getComputedStyle() trigger style recalc? Done (removed all *.offsetTop in this file). https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:30: backgroundColorShouldBe('host_open_open/div1', 'rgb(0, 0, 255)'); On 2015/08/19 05:45:24, hayato wrote: > Could you add tests for top-level shadow hosts? > > e.g. backgroundColorShouldBe('host_open_open', 'rgb(0, 0, 0)'); Done. The expected color is rgba(0, 0, 0, 0), as top-level style "div /deep/ div" doesn't match top-level <div>. https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:72: backgroundColorShouldBe('host_open_open/div1', 'rgba(0, 0, 0, 0)'); On 2015/08/19 05:45:24, hayato wrote: > Instead of adding style elements altogether, could you test separately? > > We couldn't isolate the effects of each style element if we add them at the same > time. Done. https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:426: for (Element* element = context.element; element; element = element->parentOrShadowHostElement()) { On 2015/08/19 05:45:24, hayato wrote: > Do we need to update this place also? > > FYI. https://codereview.chromium.org/1143003008/ Yes, I'll split out this part as it needs another test case. https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... File Source/core/css/resolver/SharedStyleFinder.cpp (right): https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... Source/core/css/resolver/SharedStyleFinder.cpp:269: ShadowRoot* root1 = element().containingShadowRoot(); On 2015/08/19 05:45:24, hayato wrote: > Could you explain why we need this check briefly? Previously we assumed that when 2 nodes under parents that share styles in common can share the style (see line 209), which breaks the following case: <div> #shadow-root(open) <div#div1> <div> #shadow-root(closed) <div#div2>
added a layout test. PTAL. https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/160001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:1: <!doctype html> On 2015/08/19 07:33:54, Takayoshi Kochi wrote: > On 2015/08/19 05:45:24, hayato wrote: > > Could you add a test for a selector which has two /deep/ or a combination of > > /deep/ and '::shadow'? Does it work correctly in open/closed shadow trees? > > A test for two /deep/s would make sense, and will add it in another CL. > > However, I don't think a combination of /deep/ and ::shadow needs another test, > if each are tested against all open/closed combination, it should be enough. > Do you come up with any special case that /deep/ + ::shadow combination? > As far as I grepped, I don't see any layout test that uses both in one selector. two combinators layout test is added to this CL.
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... File Source/core/css/resolver/SharedStyleFinder.cpp (right): https://codereview.chromium.org/1270313002/diff/160001/Source/core/css/resolv... Source/core/css/resolver/SharedStyleFinder.cpp:269: ShadowRoot* root1 = element().containingShadowRoot(); On 2015/08/19 07:33:54, Takayoshi Kochi wrote: > On 2015/08/19 05:45:24, hayato wrote: > > Could you explain why we need this check briefly? > > Previously we assumed that when 2 nodes under parents that share styles in > common > can share the style (see line 209), which breaks the following case: > > <div> > #shadow-root(open) > <div#div1> > <div> > #shadow-root(closed) > <div#div2> Acknowledged. https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:121: shouldBe('host_open_open.querySelectorAll("div /deep/ div").length', '3'); Could you assert the content of the result of querySelectorAll, instead of only the length of it? https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html:41: 'closedhost' It looks the order of the nodes returned by querySelectorAll is inconsistency between for open and closed shadow trees. I guess the spec needs to define the order, but from where this inconsistency came in Blink? This is expected? https://codereview.chromium.org/1270313002/diff/200001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/200001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:436: Element* element = context.element; I'm afraid that it would be difficult to read what this while loop does. Could you refactor this loop so that it would be easier to read the intention? Like before this CL https://codereview.chromium.org/1143003008/, using static inline function.
Thanks for the comments. PTAL https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinator-and-shadow-pseudo.html:121: shouldBe('host_open_open.querySelectorAll("div /deep/ div").length', '3'); On 2015/08/20 03:47:11, hayato wrote: > Could you assert the content of the result of querySelectorAll, instead of only > the length of it? Done. https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html:41: 'closedhost' On 2015/08/20 03:47:11, hayato wrote: > It looks the order of the nodes returned by querySelectorAll is inconsistency > between for open and closed shadow trees. > > I guess the spec needs to define the order, but from where this inconsistency > came in Blink? This is expected? Which do you think is inconsistent? The first 4 elements are from 'openhost' tree, whose ancestors are all open (#openhost, #openhost_in_openshadow, #div_open_open#, #closedhost_in_openshadow) and the order is in the document order. The last one is the only element that matches in the second shadow tree, as shadow host itself (#closedhost) is visible from document tree. This comes next to the previous ones, because it's appended after #openhost. https://codereview.chromium.org/1270313002/diff/200001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/200001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:436: Element* element = context.element; On 2015/08/20 03:47:11, hayato wrote: > I'm afraid that it would be difficult to read what this while loop does. > > Could you refactor this loop so that it would be easier to read the intention? > > Like before this CL https://codereview.chromium.org/1143003008/, using static > inline function. Done. https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:435: // TODO(kochi): closed mode tree should be handled as well for ::content. Added TODO for fixing this later.
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/220001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)
https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html:41: 'closedhost' You are right. My bad. :( Maybe the current naming convention made me confused. It's up to you, but the naming convention of "from-outer-to-inner, instead of "from-inner-to-outer", might be preferred to avoid confusion in this kind of selector test. https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:312: static inline Element* parentOrOpenShadowHostElement(const Element* element) This might be confusing name because shadow host's open-ness is irrelevant here. Could it have better name? https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:312: static inline Element* parentOrOpenShadowHostElement(const Element* element) const Element&
PTAL https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... File LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html (right): https://codereview.chromium.org/1270313002/diff/200001/LayoutTests/fast/dom/s... LayoutTests/fast/dom/shadow/closed-mode-deep-combinators.html:41: 'closedhost' On 2015/08/20 07:09:59, hayato wrote: > You are right. My bad. :( > > Maybe the current naming convention made me confused. > It's up to you, but the naming convention of "from-outer-to-inner, instead of > "from-inner-to-outer", might be preferred to avoid confusion in this kind of > selector test. Done. https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:312: static inline Element* parentOrOpenShadowHostElement(const Element* element) On 2015/08/20 07:09:59, hayato wrote: > const Element& Done. https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:312: static inline Element* parentOrOpenShadowHostElement(const Element* element) On 2015/08/20 07:09:59, hayato wrote: > This might be confusing name because shadow host's open-ness is irrelevant here. > Could it have better name? How about parentOrShadowHostElementWithoutExceedingClosedBoundary()?
lgtm https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... File Source/core/css/SelectorChecker.cpp (right): https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... Source/core/css/SelectorChecker.cpp:312: static inline Element* parentOrOpenShadowHostElement(const Element* element) On 2015/08/20 07:32:50, Takayoshi Kochi wrote: > On 2015/08/20 07:09:59, hayato wrote: > > This might be confusing name because shadow host's open-ness is irrelevant > here. > > Could it have better name? > > How about parentOrShadowHostElementWithoutExceedingClosedBoundary()? I would prefer parentOrShadowHostButDisallowEscapingClosedShadowTree, which was the former name. :) I'd like to avoid using a term of *boundary* in general becase the spec doesn' define it.
On 2015/08/20 10:16:29, hayato wrote: > lgtm > > https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... > File Source/core/css/SelectorChecker.cpp (right): > > https://codereview.chromium.org/1270313002/diff/220001/Source/core/css/Select... > Source/core/css/SelectorChecker.cpp:312: static inline Element* > parentOrOpenShadowHostElement(const Element* element) > On 2015/08/20 07:32:50, Takayoshi Kochi wrote: > > On 2015/08/20 07:09:59, hayato wrote: > > > This might be confusing name because shadow host's open-ness is irrelevant > > here. > > > Could it have better name? > > > > How about parentOrShadowHostElementWithoutExceedingClosedBoundary()? > > I would prefer parentOrShadowHostButDisallowEscapingClosedShadowTree, which was > the former name. :) > > I'd like to avoid using a term of *boundary* in general becase the spec doesn' > define it. Renamed as suggested. Thanks for the reviews!
The CQ bit was checked by kochi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hayato@chromium.org Link to the patchset: https://codereview.chromium.org/1270313002/#ps260001 (title: "Rename function as suggested for land")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/260001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by kochi@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1270313002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1270313002/260001
Message was sent while issue was closed.
Committed patchset #8 (id:260001) as https://src.chromium.org/viewvc/blink?view=rev&revision=200909 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
