New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 853441 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Compat



Sign in to add a comment

[css-contain] Size containment causes replaced elements intrinsic size to be treated as zero

Reported by goo...@gtalbot.org, Jun 16 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

Example URL:
http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.xht

Steps to reproduce the problem:
Load these 2 reduced and self-explanatory tests:

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-042.xht

What is the expected behavior?
5678 should be rendered without digits overlapping themselves

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041-ref.xht

What went wrong?
5678 is rendered and digits overlap themselves

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 68.0.3440.15  Channel: dev
OS Version: 
Flash Version: irrelevant

- "(...) if the element is an internal table element (...), size containment has no effect."
https://www.w3.org/TR/css-contain-1/#containment-size
- Florian Rivoal should be added to CC list
- Suggested Component: Blink>Layout
 
Labels: Needs-Triage-M68
Components: Blink>CSS
Labels: Triaged-ET Needs-Bisect OS-Mac OS-Windows
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Mac 10.13.3, Win-10 and Ubuntu 17.10 using chrome reported version #68.0.3440.15 but the same is not reproducible in the latest canary #69.0.3463.0.

Will provide the reverse bisect info soon and marking it as untriaged.

Thanks...!!

Comment 3 by goo...@gtalbot.org, Jun 18 2018

> he same is not reproducible in the latest canary #69.0.3463.0.

Then, this may be related to recent work inside  Issue 843329  . 

Cc: r...@igalia.com krajshree@chromium.org
Components: -Blink>CSS Blink>Layout
Labels: Needs-Feedback
reporter@ - Tried checking the issue using the same url: http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.xht but now issue is seen on both chrome reported version #68.0.3440.15 and latest canary #69.0.3463.0. Same behaviour is observed in firefox also. Hence, it won't be possible to provide bisect info from our end.

Requesting the reporter to please provide any other sample url to triage the issue further from TE-end.
ccing rego@ from issue id: 843329 for further inputs on this issue.

Thanks...!!
853441@firefox.png
120 KB View Download
853441@chrome.png
316 KB View Download

Comment 5 by r...@igalia.com, Jun 19 2018

Cc: flor...@rivoal.net
Status: Available (was: Untriaged)
Summary: [css-contain] Size containment causes replaced elements intrinsic size to be treated as zero (was: [css-contain] size containment should have no effect on internal table elements (table-cell))
Mmm, internal table elements have been fixed as part of  issue #843329 .

However on the links I don't see anything related to internal table elements
but about intrinsic size of replaced elements.

It seems that the URLs have changed, as there are no digits or something like that.
So I'm going to attach the current examples as the URLs might change again in the future and rename the bug report.

@gtalbot these look like tests for the WPT test suite,
are you planing to upload them there?

Comment 6 by r...@igalia.com, Jun 19 2018

contain-size-041.xht
1.5 KB Download
contain-size-042.xht
1.5 KB Download

Comment 7 by goo...@gtalbot.org, Jun 19 2018

> reporter@ - Tried checking the issue using the same url: http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.xht 
> but now issue is seen on 
> both chrome reported version #68.0.3440.15 and latest canary #69.0.3463.0. 
> Same behaviour is observed in firefox also. Hence, it won't be possible to 
> provide bisect info from our end.

Krajshree, you are correct. I am sorry. I had to renumber the filenames and I forgot about this Issue over here.
The table-cell test is now at:

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-051.xht

> @gtalbot these look like tests for the WPT test suite,
> are you planing to upload them there?

Yes, I am planning to submit them to the WPT test suite eventually. Manuel Rego, I renumbered my tests so that they do not "collide" and would not "collide" with your filename-numbers.


Comment 8 by goo...@gtalbot.org, Jun 19 2018

My 'contain: size' tests and their description (as of today):

https://bugzilla.mozilla.org/show_bug.cgi?id=1463594#c5

Comment 9 by goo...@gtalbot.org, Jun 19 2018

> internal table elements have been fixed as part of   issue #843329  .

Then we can close this Issue as WORKSFORME .

Comment 10 by r...@igalia.com, Jun 19 2018

> Yes, I am planning to submit them to the WPT test suite eventually. Manuel Rego, I renumbered my tests so that they do not "collide" and would not "collide" with your filename-numbers.

Ok, that' really nice. It'd be really nice to have your tests in WPT.

> Then we can close this Issue as WORKSFORME .

I've renamed the issue and as size containment is not working fine for replaced elements,
as it's shown by your tests:
* http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.xht
* http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-042.xht

So I think we can keep it open, another option is to close it and report a new one.

Comment 11 by goo...@gtalbot.org, Jun 19 2018

> I've renamed the issue (...)
> we can keep it open

Okay, great! Thanks!

Comment 12 by r...@igalia.com, Jun 22 2018

Labels: -Needs-Feedback -Needs-Bisect
Owner: r...@igalia.com
Status: Started (was: Available)
Ok, I'm working on a patch to fix this, I'll be writing my own test at this stage.

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf0137c1ac3be7ceddd4506e397249ec54701af9

commit cf0137c1ac3be7ceddd4506e397249ec54701af9
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Fri Jun 22 14:31:11 2018

[css-contain] Size containment replaced elements intrinsic size is zero

The spec is very clear about this
(https://drafts.csswg.org/css-contain/#containment-size):
  "Replaced elements must be treated as having an intrinsic width
   and height of 0."

The patch modifies LayoutReplaced::ComputeIntrinsicSizingInfo()
so that it returns zero for elements with "contain: size".

BUG= 853441 
TEST=external/wpt/css/css-contain/contain-size-013.html

Change-Id: I91490b120a110be20a91d564da4fdc3d9a11b70a
Reviewed-on: https://chromium-review.googlesource.com/1110819
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#569610}
[add] https://crrev.com/cf0137c1ac3be7ceddd4506e397249ec54701af9/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-013.html
[modify] https://crrev.com/cf0137c1ac3be7ceddd4506e397249ec54701af9/third_party/blink/renderer/core/layout/layout_replaced.cc

Comment 14 by r...@igalia.com, Jun 22 2018

Status: Fixed (was: Started)
This should be fixed now.
I must keep only 1 copy of tests and reference files on my site. The links given in first comments of this issue will be broken.

If you are looking for the tests and reference files for this issue, they are now at:

Tests on 'contain: size' and inline replaced elements:

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-041.html

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-042.html


Associated reference files:

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-021-ref.html

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/contain-size-022-ref.html

JFTR the tests linked in the last comment are working fine now.

Sign in to add a comment