-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-grid] Add test to verify the minimum size of images #5337
[css-grid] Add test to verify the minimum size of images #5337
Conversation
Notifying @atanassov, @fantasai, @jxs, @plinss, @tabatkins, and @tomalec. (Learn how reviewing works.) |
*This report has been truncated because the total content is 547482 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Firefox (nightly channel)Testing web-platform-tests at revision 8e885f9 All results98 tests ran/XMLHttpRequest/event-readystatechange-loaded.htm
/beacon/headers/header-referrer-no-referrer-when-downgrade.https.html
/beacon/headers/header-referrer-no-referrer.html
/beacon/headers/header-referrer-origin-when-cross-origin.html
/beacon/headers/header-referrer-origin.html
/beacon/headers/header-referrer-same-origin.html
/beacon/headers/header-referrer-strict-origin-when-cross-origin.https.html
/beacon/headers/header-referrer-strict-origin.https.html
/beacon/headers/header-referrer-unsafe-url.https.html
/css-timing-1/cubic-bezier-timing-functions-output.html
/css-timing-1/frames-timing-functions-output.html
/css-timing-1/frames-timing-functions-syntax.html
/css-timing-1/step-timing-functions-output.html
/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html
/dom/lists/DOMTokenList-iteration.html
/geolocation-API/PositionOptions.https.html
/geolocation-API/getCurrentPosition_IDL.https.html
/geolocation-API/getCurrentPosition_permission_allow.https.html
/geolocation-API/getCurrentPosition_permission_deny.https.html
/geolocation-API/watchPosition_permission_deny.https.html
/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
/html/semantics/forms/textfieldselection/selection-start-end.html
/html/semantics/forms/textfieldselection/selection-value-interactions.html
/html/semantics/forms/textfieldselection/selection.html
/html/semantics/forms/textfieldselection/textfieldselection-setRangeText.html
/html/semantics/forms/textfieldselection/textfieldselection-setSelectionRange.html
|
*This report has been truncated because the total content is 1350566 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Chrome (unstable channel)Testing web-platform-tests at revision 8e885f9 Unstable results
|
I'm using Chrome 59.0.3053.3 and the test passes locally. |
@mrego: I understood that the current spec text is going to be changed, why do you say the current text is correct? |
@mrego The problem isn't the failure, it's the inconsistent in the results in both Firefox and Chrome. As I said before, I think this is to do with image loading. Consider:
This is parsed, and a request for support/50x50-green.png is initiated.
This script is then parsed and run, leading to it fail because #img-1's computed style is the default width/height of an unloaded image. The image then loads, but it's too late because the test has already run. This is why I think its results are random in both Firefox and Chrome. There's a race condition in here and that needs dealt with (with the |
I meant that this test is currently right, until the spec is not modified. That should happen soon, but we never know when it's going to happen exactly. So I was wondering if it'd make sense to merge this first and later update it once the spec is modified. Waiting for the spec to be revised works for me too.
Yeah sorry I was not getting it, it's clear there are issues with the test, I'll try to get them fixed. Thanks! |
These tests are now available on w3c-test.org |
opened #5408 for the Travis failure |
9c3117c
to
91ab32a
Compare
Firefox (nightly)Testing web-platform-tests at revision c1ca5ae All results1 test ran/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html
|
Sauce (safari)Testing web-platform-tests at revision c1ca5ae All results1 test ran/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html
|
Chrome (unstable)Testing web-platform-tests at revision c1ca5ae All results1 test ran/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html
|
Sauce (MicrosoftEdge)Testing web-platform-tests at revision c1ca5ae All results1 test ran/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html
|
And almost 2 months have passe and the spec has not been updated yet. |
OK. Let's go ahead, but we should keep an eye on it. |
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the automatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
grid-items/grid-minimum-size-grid-items-021.html was flacky on the different browsers as sometimes they run the checks before the images were loaded. This patch fixes this issue waiting for the "load" event before running them.
91ab32a
to
a54be81
Compare
This is a test created from issue w3c/csswg-drafts#1117.
The test checks how the automatic minimum size of images affect
to the grid container sizing and the track sizes.
The test also verifies the size of the image in the different cases.
The test combines the following cases:
@svillar after the discussion in w3c/csswg-drafts#1117 it seems that per current spec this test is right.
Maybe we can land it as is and update it when the spec changes, or do you think it's better to wait until the spec is modified?