Bug 178073 - Don't use intrinsic width if our container's width is zero
Summary: Don't use intrinsic width if our container's width is zero
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: BlinkMergeCandidate, InRadar
Depends on:
Blocks: 178337
  Show dependency treegraph
 
Reported: 2017-10-09 01:13 PDT by Tim Horton
Modified: 2017-10-16 05:12 PDT (History)
12 users (show)

See Also:


Attachments
testcase (604 bytes, text/html)
2017-10-09 01:13 PDT, Tim Horton
no flags Details
screenshot of safari (bad case) (27.34 KB, image/png)
2017-10-09 01:14 PDT, Tim Horton
no flags Details
Another example to reproduce the issue (458 bytes, text/html)
2017-10-09 06:58 PDT, Manuel Rego Casasnovas
no flags Details
Simplified test case (251 bytes, text/html)
2017-10-09 07:03 PDT, Manuel Rego Casasnovas
no flags Details
Patch (294.73 KB, patch)
2017-10-13 06:02 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (2.03 MB, application/zip)
2017-10-13 06:59 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (2.30 MB, application/zip)
2017-10-13 07:12 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.26 MB, application/zip)
2017-10-13 07:35 PDT, Build Bot
no flags Details
Patch (667.45 KB, patch)
2017-10-13 09:21 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-elcapitan (1.90 MB, application/zip)
2017-10-13 10:52 PDT, Build Bot
no flags Details
Patch for landing (668.22 KB, patch)
2017-10-16 04:23 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2017-10-09 01:13:36 PDT
Steps to Reproduce:

1. Using the attached test case, resize your browser window to the minimum width.

Expected:

The SVG line should not be visible.

Actual:

The SVG line discontinuously jumps to overlap the text in the next grid column when the width reaches 0.

Notes:

Chrome and Firefox both behave more sensibly (though they do not agree about the correct behavior).
Comment 1 Tim Horton 2017-10-09 01:13:48 PDT
Created attachment 323160 [details]
testcase
Comment 2 Tim Horton 2017-10-09 01:14:23 PDT
Created attachment 323161 [details]
screenshot of safari (bad case)
Comment 3 Manuel Rego Casasnovas 2017-10-09 06:58:32 PDT
Created attachment 323172 [details]
Another example to reproduce the issue


The problem is easily reproducible with an image too.
Check the new attached example, the size of the image depends on the "1fr" column. When it's very close to 0, the image is size 100% of the column breadth, but when it goes to 0 then the image uses the intrisic size for width (100px in the example).
In Chromium and Firefox the image just disppears when the column size is 0px.
We need to investigate what's going on with that image, and why it's not using the 0px breadth of the column to resolve its 100% width.

The differences between Firefox and Chromium in the first example might be related to the intrinsic size of the SVG or something like that.
Comment 4 Manuel Rego Casasnovas 2017-10-09 07:03:20 PDT
Created attachment 323173 [details]
Simplified test case

This last example is even more simple, just a grid with a column of 0px.
Then the image has a 100% width and is inside a grid item on the first column.
The width of the image should be 0px but it's 100px.
Comment 5 Manuel Rego Casasnovas 2017-10-13 04:52:36 PDT
Ok so the issue is not related to Grid Layout but is a generic issue for replaced elements.

It has been fixed a while ago in Blink: https://codereview.chromium.org/253743008/
But it seems the patch was never imported into WebKit.

The patch is quite small so I guess we can easily import it.
Comment 6 Manuel Rego Casasnovas 2017-10-13 06:02:02 PDT
Created attachment 323664 [details]
Patch
Comment 7 Manuel Rego Casasnovas 2017-10-13 06:03:16 PDT
The patch is missing the Mac baselines for width100percent-image.html but I'll get them from the EWS.
Comment 8 Build Bot 2017-10-13 06:59:16 PDT
Comment on attachment 323664 [details]
Patch

Attachment 323664 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4844499

New failing tests:
fast/replaced/width100percent-image.html
Comment 9 Build Bot 2017-10-13 06:59:17 PDT
Created attachment 323665 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 10 Build Bot 2017-10-13 07:11:58 PDT
Comment on attachment 323664 [details]
Patch

Attachment 323664 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4844481

New failing tests:
fast/replaced/width100percent-image.html
Comment 11 Build Bot 2017-10-13 07:12:00 PDT
Created attachment 323666 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 12 Build Bot 2017-10-13 07:35:12 PDT
Comment on attachment 323664 [details]
Patch

Attachment 323664 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4844561

New failing tests:
fast/replaced/width100percent-image.html
Comment 13 Build Bot 2017-10-13 07:35:13 PDT
Created attachment 323669 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 14 Manuel Rego Casasnovas 2017-10-13 09:21:14 PDT
Created attachment 323680 [details]
Patch
Comment 15 Build Bot 2017-10-13 10:52:12 PDT
Comment on attachment 323680 [details]
Patch

Attachment 323680 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4846322

New failing tests:
http/tests/loading/basic-credentials-sent-automatically.html
Comment 16 Build Bot 2017-10-13 10:52:14 PDT
Created attachment 323702 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 17 Sergio Villar Senin 2017-10-16 03:38:02 PDT
Comment on attachment 323680 [details]
Patch

Makes sense. Bonus points for interoperability
Comment 18 Manuel Rego Casasnovas 2017-10-16 04:23:38 PDT
Created attachment 323889 [details]
Patch for landing
Comment 19 WebKit Commit Bot 2017-10-16 05:10:54 PDT
Comment on attachment 323889 [details]
Patch for landing

Clearing flags on attachment: 323889

Committed r223389: <https://trac.webkit.org/changeset/223389>
Comment 20 WebKit Commit Bot 2017-10-16 05:10:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 21 Radar WebKit Bug Importer 2017-10-16 05:12:58 PDT
<rdar://problem/35005193>