Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1587023002:
SVG with zero intrinsic size should be respected always (Closed)
|
Created:
4 years, 11 months ago by Shanmuga Pandi Modified:
4 years, 10 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSVG with zero intrinsic size should be respected always.
Earlier it was assumed that a zero intrinsic size means no intrinsic size.
BUG=363949
Committed: https://crrev.com/da94afcf81b0564c60b759895245011b42662d7b
Cr-Commit-Position: refs/heads/master@{#373475}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Align with review comments #
Total comments: 7
Patch Set 3 : Used LengthSize instead of FloatSize #Patch Set 4 : Align with review comments #Patch Set 5 : Added TODO #
Messages
Total messages: 35 (7 generated)
Description was changed from ========== SVG with zero intrinsic size should be respected always Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 ========== to ========== SVG with zero intrinsic size should be respected always. Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 ==========
shanmuga.m@samsung.com changed reviewers: + rob.buis@samsung.com
PTAL!! Thanks
Looks good to me.
shanmuga.m@samsung.com changed reviewers: + ed@chromium.org, fs@opera.com, pdr@chromium.org
fs@opera.com changed reviewers: + davve@opera.com
+davve
https://codereview.chromium.org/1587023002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): https://codereview.chromium.org/1587023002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:286: hasIntrinsicWidth = contentLayoutObject->style()->logicalWidth().isFixed(); You rely on the (logical) intrinsic width being collected as presentation attribute, thus ending up in style()->logicalWidth(). Then you use style()->logicalWidth() as indication whether there is an intrinsic (logical) width or not. A weakness of this trick is that there can be "proper" style on the SVG that shouldn't indicate intrinsic width (or height respectively). The example below gives a 300x150 green rect in Firefox and Chrome and I think that's correct. With your patch we would think there is a (zero) intrinsic width and render nothing. html: <object style="background: green" data="some-svg.svg"></object> some-svg.svg: <svg xmlns="http://www.w3.org/2000/svg" style="width: 0"></svg> I think we need something more elaborate here, preferable by propagating enough information from SVGSVGElement::intrinsicWidth/Height to make the correct hasIntrinsicWidth/Height judgement in LayoutReplaced::computeReplacedLogicalWidth/Height().
On 2016/01/18 at 15:52:48, davve wrote: ... > I think we need something more elaborate here, preferable by > propagating enough information from > SVGSVGElement::intrinsicWidth/Height to make the correct > hasIntrinsicWidth/Height judgement in > LayoutReplaced::computeReplacedLogicalWidth/Height(). Maybe SVGSVGElement::intrinsicWidth/Height could return Length(Auto) (aka Length()) for the "no intrinsic width/height" case and then that could be propagated up using LengthSize or something like that. Quite a bit of type-wrangling there...
Done the changes. Please review it. Thanks https://codereview.chromium.org/1587023002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): https://codereview.chromium.org/1587023002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:286: hasIntrinsicWidth = contentLayoutObject->style()->logicalWidth().isFixed(); > html: > > <object style="background: green" data="some-svg.svg"></object> > > some-svg.svg: > > <svg xmlns="http://www.w3.org/2000/svg" style="width: 0"></svg> > > I think we need something more elaborate here, preferable by > propagating enough information from > SVGSVGElement::intrinsicWidth/Height to make the correct > hasIntrinsicWidth/Height judgement in > LayoutReplaced::computeReplacedLogicalWidth/Height(). Instead of LogicalWidth()/Height(), getting hasIntrinsicWidth()/Height() to detect whether SVG has intrisic width/height. This solve the above the problem and the bug reported.
https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html:4: <!--There should not be any red rectangle visible. --> Not sure why you have all these SVGs that's not supposed to be visible in the -expected file. Worst case, both <object> and <svg> regress at the same time, in the same way, making the reftest still pass. In general, an expected file should be written as simple as possible (and maybe with different primitives). If you want to test zero dimensions inline SVGs, I'd suggest you add a separate test for that, or make the test more general and test that as well. https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height.html:15: <object data="data:image/svg+xml, Missing a space? https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:306: bool hasIntrinsicWidth = constrainedSize.width() > 0 || hasIntrinsicWidthForLayoutBox(contentLayoutObject); While I think this is safer than the previous fix, I still think we should keep this information propagation within the LayoutBox::computeIntrinsicRatioInformation API somehow. Did you investigate fs' idea?
> https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:306: bool > hasIntrinsicWidth = constrainedSize.width() > 0 || > hasIntrinsicWidthForLayoutBox(contentLayoutObject); > While I think this is safer than the previous fix, I still think we should keep > this information propagation within the > LayoutBox::computeIntrinsicRatioInformation API somehow. Did you investigate fs' > idea? I will check it further as per fs suggestion. https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html:4: <!--There should not be any red rectangle visible. --> On 2016/01/20 07:54:22, David Vest wrote: > Not sure why you have all these SVGs that's not supposed to be visible in the > -expected file. Worst case, both <object> and <svg> regress at the same time, in > the same way, making the reftest still pass. > > In general, an expected file should be written as simple as possible (and maybe > with different primitives). If you want to test zero dimensions inline SVGs, I'd > suggest you add a separate test for that, or make the test more general and test > that as well. Ah. My intention of this file is just as the output should be same as <object> and <svg> with zero intrinsic. I should keep -expected.html as just empty with just <body> </body>. Will it be fine ?
https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html:4: <!--There should not be any red rectangle visible. --> On 2016/01/20 13:41:40, Shanmuga Pandi wrote: > On 2016/01/20 07:54:22, David Vest wrote: > > Not sure why you have all these SVGs that's not supposed to be visible in the > > -expected file. Worst case, both <object> and <svg> regress at the same time, > in > > the same way, making the reftest still pass. > > > > In general, an expected file should be written as simple as possible (and > maybe > > with different primitives). If you want to test zero dimensions inline SVGs, > I'd > > suggest you add a separate test for that, or make the test more general and > test > > that as well. > > Ah. My intention of this file is just as the output should be same as <object> > and <svg> with zero intrinsic. > I should keep -expected.html as just empty with just <body> </body>. Will it be > fine ? If you just want an empty file '<!DOCTYPE html>' would do.
> https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): > > https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:306: bool > hasIntrinsicWidth = constrainedSize.width() > 0 || > hasIntrinsicWidthForLayoutBox(contentLayoutObject); > While I think this is safer than the previous fix, I still think we should keep > this information propagation within the > LayoutBox::computeIntrinsicRatioInformation API somehow. Did you investigate fs' > idea? @davve, I have two approach in mind. 1. Passing LengthSize instead of FloatSize to LayoutBox::computeIntrinsicRatioInformation. It requires many changes(type conversion to/from FloatSize and LengthSize and setting LengthSize type as "Auto" if width/height is empty, except LayoutSVGRoot case (it will set based on SVG intrinsic). Based on LenghType, LayoutReplaced will decide hasIntrisicWidth/Height or not. OR 2. Only Just changing LengthSize instead of FloatSize to LayoutReplaced::computeIntrinsicRationInformation. It requires minimal changes, and we set Length type based on width/height value or SVG hasIntrinsicWidth/Height. Please suggest me , which approach will be better ? Thanks.
On 2016/01/25 11:23:25, Shanmuga Pandi wrote: > > > https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): > > > > > https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > > third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:306: bool > > hasIntrinsicWidth = constrainedSize.width() > 0 || > > hasIntrinsicWidthForLayoutBox(contentLayoutObject); > > While I think this is safer than the previous fix, I still think we should > keep > > this information propagation within the > > LayoutBox::computeIntrinsicRatioInformation API somehow. Did you investigate > fs' > > idea? > > @davve, > > I have two approach in mind. > > 1. Passing LengthSize instead of FloatSize to > LayoutBox::computeIntrinsicRatioInformation. > It requires many changes(type conversion to/from FloatSize and LengthSize > and setting LengthSize type as "Auto" if width/height is empty, except > LayoutSVGRoot case (it will set based on SVG intrinsic). > Based on LenghType, LayoutReplaced will decide hasIntrisicWidth/Height or > not. This indeed sounds like a lot of churn for this relatively simple thing. But if we're going to have a state for "no intrinsic width/height" separate from "zero intrinsic width/height" we must propagate more information somehow. > OR > > 2. Only Just changing LengthSize instead of FloatSize to > LayoutReplaced::computeIntrinsicRationInformation. > It requires minimal changes, and we set Length type based on width/height > value or SVG hasIntrinsicWidth/Height. I'm not sure I understand (2). LayoutReplaced::computeIntrinsicRatioInformation is not used when there is a embeddedContentBox()?
On 2016/01/25 13:22:55, David Vest wrote: > On 2016/01/25 11:23:25, Shanmuga Pandi wrote: > > > > > > https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > > > File third_party/WebKit/Source/core/layout/LayoutReplaced.cpp (right): > > > > > > > > > https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Sour... > > > third_party/WebKit/Source/core/layout/LayoutReplaced.cpp:306: bool > > > hasIntrinsicWidth = constrainedSize.width() > 0 || > > > hasIntrinsicWidthForLayoutBox(contentLayoutObject); > > > While I think this is safer than the previous fix, I still think we should > > keep > > > this information propagation within the > > > LayoutBox::computeIntrinsicRatioInformation API somehow. Did you investigate > > fs' > > > idea? > > > > @davve, > > > > I have two approach in mind. > > > > 1. Passing LengthSize instead of FloatSize to > > LayoutBox::computeIntrinsicRatioInformation. > > It requires many changes(type conversion to/from FloatSize and LengthSize > > and setting LengthSize type as "Auto" if width/height is empty, except > > LayoutSVGRoot case (it will set based on SVG intrinsic). > > Based on LenghType, LayoutReplaced will decide hasIntrisicWidth/Height or > > not. > > This indeed sounds like a lot of churn for this relatively simple > thing. But if we're going to have a state for "no intrinsic > width/height" separate from "zero intrinsic width/height" we must > propagate more information somehow. > We can propagate this info from LayoutSVGRoot --> SVGSVGElement --> intrinsicWidth/Height. But I am not sure about other LayoutBox object. (Looks like we need to propagate the similar info from only LayoutImage) Correct me If I am wrong. And as for as i see embeddedContentBox() can be LayoutSVGRoot. Anyother LayoutObject can be embeddedContentBox? so that I can understand better. > > OR > > > > 2. Only Just changing LengthSize instead of FloatSize to > > LayoutReplaced::computeIntrinsicRationInformation. > > It requires minimal changes, and we set Length type based on width/height > > value or SVG hasIntrinsicWidth/Height. > > I'm not sure I understand (2). > LayoutReplaced::computeIntrinsicRatioInformation is not used when > there is a embeddedContentBox()? Oh Sorry for typo error for (2). What I meant is LayoutReplaced::computeAspectRatioInformationForLayoutBox. We do pass LengthSize instead of FloatSize. Based on LengthSize type, LayoutReplaced::computeReplacedLogicalWidth/Height will decide hasIntrisicWidth/Height or not. But this approach sounds similar to the latest patch which I submitted except moving the condition in different place. Sample: ~~~~~~~~ LengthSize constrainedLength; computeAspectRatioInformationForLayoutBox(contentLayoutObject, constrainedLength, intrinsicRatio); bool hasIntrinsicWidth = !constrainedLength.width().isAuto(); bool hasIntrinsicHeight = !constrainedLength.height().isAuto();
> We can propagate this info from LayoutSVGRoot --> SVGSVGElement --> > intrinsicWidth/Height. > But I am not sure about other LayoutBox object. (Looks like we need to propagate > the similar info from only LayoutImage) > Correct me If I am wrong. Don't think we need to touch LayoutImage at this point. > And as for as i see embeddedContentBox() can be LayoutSVGRoot. > Anyother LayoutObject can be embeddedContentBox? so that I can understand > better. Yes, only LayoutSVGRoot can be embeddedContentBox() currently (and for the foreseeable future.) > > > OR > > > > > > 2. Only Just changing LengthSize instead of FloatSize to > > > LayoutReplaced::computeIntrinsicRationInformation. > > > It requires minimal changes, and we set Length type based on width/height > > > value or SVG hasIntrinsicWidth/Height. > > > > I'm not sure I understand (2). > > LayoutReplaced::computeIntrinsicRatioInformation is not used when > > there is a embeddedContentBox()? > > > Oh Sorry for typo error for (2). > What I meant is LayoutReplaced::computeAspectRatioInformationForLayoutBox. > We do pass LengthSize instead of FloatSize. > Based on LengthSize type, LayoutReplaced::computeReplacedLogicalWidth/Height > will decide hasIntrisicWidth/Height or > not. > But this approach sounds similar to the latest patch which I submitted except > moving the condition in different place. > > Sample: > ~~~~~~~~ > LengthSize constrainedLength; > computeAspectRatioInformationForLayoutBox(contentLayoutObject, > constrainedLength, intrinsicRatio); > bool hasIntrinsicWidth = !constrainedLength.width().isAuto(); > bool hasIntrinsicHeight = !constrainedLength.height().isAuto(); And let computeAspectRatioInformationForLayoutBox gain knowledge about SVGSVGElement (We still won't get rid of including SVGSVGElement in LayoutReplaced)? I agree that's similar to current patch. Maybe it's worth poking a hole for this, if the other solution is too convoluted. fs?
On 2016/01/25 at 17:01:50, davve wrote: ... > > > > OR > > > > > > > > 2. Only Just changing LengthSize instead of FloatSize to > > > > LayoutReplaced::computeIntrinsicRationInformation. > > > > It requires minimal changes, and we set Length type based on width/height > > > > value or SVG hasIntrinsicWidth/Height. > > > > > > I'm not sure I understand (2). > > > LayoutReplaced::computeIntrinsicRatioInformation is not used when > > > there is a embeddedContentBox()? > > > > > > Oh Sorry for typo error for (2). > > What I meant is LayoutReplaced::computeAspectRatioInformationForLayoutBox. > > We do pass LengthSize instead of FloatSize. > > Based on LengthSize type, LayoutReplaced::computeReplacedLogicalWidth/Height > > will decide hasIntrisicWidth/Height or > > not. > > But this approach sounds similar to the latest patch which I submitted except > > moving the condition in different place. > > > > Sample: > > ~~~~~~~~ > > LengthSize constrainedLength; > > computeAspectRatioInformationForLayoutBox(contentLayoutObject, > > constrainedLength, intrinsicRatio); > > bool hasIntrinsicWidth = !constrainedLength.width().isAuto(); > > bool hasIntrinsicHeight = !constrainedLength.height().isAuto(); > > And let computeAspectRatioInformationForLayoutBox gain knowledge about > SVGSVGElement (We still won't get rid of including SVGSVGElement in > LayoutReplaced)? I agree that's similar to current patch. Maybe it's > worth poking a hole for this, if the other solution is too convoluted. fs? I think I would prefer less poking of holes (a/the more invasive solution is preferably done in smaller steps anyway so.) I guess we can see what it look likes though...
Please check this. Thanks https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height-expected.html:4: <!--There should not be any red rectangle visible. --> On 2016/01/20 16:05:19, David Vest wrote: > On 2016/01/20 13:41:40, Shanmuga Pandi wrote: > > On 2016/01/20 07:54:22, David Vest wrote: > > > Not sure why you have all these SVGs that's not supposed to be visible in > the > > > -expected file. Worst case, both <object> and <svg> regress at the same > time, > > in > > > the same way, making the reftest still pass. > > > > > > In general, an expected file should be written as simple as possible (and > > maybe > > > with different primitives). If you want to test zero dimensions inline SVGs, > > I'd > > > suggest you add a separate test for that, or make the test more general and > > test > > > that as well. > > > > Ah. My intention of this file is just as the output should be same as <object> > > and <svg> with zero intrinsic. > > I should keep -expected.html as just empty with just <body> </body>. Will it > be > > fine ? > > If you just want an empty file '<!DOCTYPE html>' would do. Done. https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height.html (right): https://codereview.chromium.org/1587023002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/custom/object-sizing-zero-intrinsic-width-height.html:15: <object data="data:image/svg+xml, On 2016/01/20 07:54:22, David Vest wrote: > Missing a space? Done.
On 2016/01/28 at 11:21:57, shanmuga.m wrote: > Please check this. I think that PS3 keeps all the "ugliness" of PS2 but adds more code in the direction of a "proper" fix. I guess I could be convinced of going with a PS2-like fix with cleanup as follow-up.
On 2016/01/29 11:26:42, fs wrote: > On 2016/01/28 at 11:21:57, shanmuga.m wrote: > > Please check this. > > I think that PS3 keeps all the "ugliness" of PS2 but adds more code in the > direction of a "proper" fix. I guess I could be convinced of going with a > PS2-like fix with cleanup as follow-up. @fs, Thanks for your opinion. I will make PS2 patch and submit again. @davve, Your opinion ?
On 2016/01/29 13:43:19, Shanmuga Pandi wrote: > @davve, > Your opinion ? Agreed. Landing the bug fix and cleaning up as a follow-up sounds good to me.
On 2016/01/30 08:55:25, David Vest wrote: > On 2016/01/29 13:43:19, Shanmuga Pandi wrote: > > > @davve, > > Your opinion ? > > Agreed. Landing the bug fix and cleaning up as a follow-up sounds good to me. And I also prefer ps#2.
On 2016/01/30 at 08:56:44, davve wrote: > On 2016/01/30 08:55:25, David Vest wrote: > > On 2016/01/29 13:43:19, Shanmuga Pandi wrote: > > > > > @davve, > > > Your opinion ? > > > > Agreed. Landing the bug fix and cleaning up as a follow-up sounds good to me. > > And I also prefer ps#2. So ps4 lgty?
On 2016/02/01 10:16:33, fs wrote: > On 2016/01/30 at 08:56:44, davve wrote: > > On 2016/01/30 08:55:25, David Vest wrote: > > > On 2016/01/29 13:43:19, Shanmuga Pandi wrote: > > > > > > > @davve, > > > > Your opinion ? > > > > > > Agreed. Landing the bug fix and cleaning up as a follow-up sounds good to > me. > > > > And I also prefer ps#2. > > So ps4 lgty? Sure, with a TODO that says we should some day pass this information through LayoutBox::computeIntrinsicRatioInformation().
On 2016/02/01 10:29:48, David Vest wrote: > On 2016/02/01 10:16:33, fs wrote: > > On 2016/01/30 at 08:56:44, davve wrote: > > > On 2016/01/30 08:55:25, David Vest wrote: > > > > On 2016/01/29 13:43:19, Shanmuga Pandi wrote: > > > > > > > > > @davve, > > > > > Your opinion ? > > > > > > > > Agreed. Landing the bug fix and cleaning up as a follow-up sounds good to > > me. > > > > > > And I also prefer ps#2. > > > > So ps4 lgty? > > Sure, with a TODO that says we should some day pass this information through > LayoutBox::computeIntrinsicRatioInformation(). Updated with TODO!! Please check!
On 2016/02/03 05:58:14, Shanmuga Pandi wrote: > Updated with TODO!! Please check! Non-owner LGTM.
lgtm
The CQ bit was checked by shanmuga.m@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1587023002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1587023002/80001
Message was sent while issue was closed.
Description was changed from ========== SVG with zero intrinsic size should be respected always. Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 ========== to ========== SVG with zero intrinsic size should be respected always. Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== SVG with zero intrinsic size should be respected always. Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 ========== to ========== SVG with zero intrinsic size should be respected always. Earlier it was assumed that a zero intrinsic size means no intrinsic size. BUG=363949 Committed: https://crrev.com/da94afcf81b0564c60b759895245011b42662d7b Cr-Commit-Position: refs/heads/master@{#373475} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/da94afcf81b0564c60b759895245011b42662d7b Cr-Commit-Position: refs/heads/master@{#373475} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
