Bug 160740

Summary: slot doesn't work as a flex container
Product: WebKit Reporter: Eric <ericbidelman>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, jan, koivisto, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=172113
Bug Depends on:    
Bug Blocks: 148695    
Attachments:
Description Flags
Test case
none
patch
none
patch
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Archive of layout-test-results from ews116 for mac-yosemite
none
patch
rniwa: review+
patch for landing none

Eric
Reported 2016-08-10 11:21:58 PDT
I'd expect slotted spans to be flex children in the following code: <nav> <span>Hello</span> <span>world</span> </nav> var nav = document.querySelector('nav'); nav.attachShadow({mode:'open'}).innerHTML = '<style>' + ':host { display: flex }' + '::slotted(span) { flex: 1 }' + '</style>' + '<slot></slot>'; Repro: http://jsbin.com/dukapa/2/edit?html,output In Safari TP, "Hello" and "world" do not flex as expected. In Blink, they do.
Attachments
Test case (874 bytes, text/html)
2016-11-03 19:06 PDT, Ryosuke Niwa
no flags
patch (19.61 KB, patch)
2016-11-14 13:58 PST, Antti Koivisto
no flags
patch (19.51 KB, patch)
2016-11-14 14:20 PST, Antti Koivisto
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite (1.03 MB, application/zip)
2016-11-14 15:26 PST, Build Bot
no flags
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.18 MB, application/zip)
2016-11-14 15:32 PST, Build Bot
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (deleted)
2016-11-14 15:37 PST, Build Bot
no flags
Archive of layout-test-results from ews116 for mac-yosemite (1.51 MB, application/zip)
2016-11-14 15:42 PST, Build Bot
no flags
patch (19.67 KB, patch)
2016-11-14 16:03 PST, Antti Koivisto
rniwa: review+
patch for landing (20.32 KB, patch)
2016-11-15 10:49 PST, Antti Koivisto
no flags
Ryosuke Niwa
Comment 1 2016-10-03 23:58:56 PDT
The problem here is that somehow slot is getting a RenderBlock.
Radar WebKit Bug Importer
Comment 2 2016-10-03 23:59:15 PDT
Ryosuke Niwa
Comment 3 2016-11-03 19:06:52 PDT
Created attachment 293846 [details] Test case
Ryosuke Niwa
Comment 4 2016-11-03 19:12:55 PDT
The problem here is that StyleResolver::adjustRenderStyle mutates display value based on the parent render style even if the current render object was display: contents. You can see this in line 898 of StyleResolver.cpp where it says: if (parentStyle.isDisplayFlexibleOrGridBox()) { style.setFloating(NoFloat); style.setDisplay(equivalentBlockDisplay(style.display(), style.isFloating(), !document().inQuirksMode())); } Even if we added style.display() != CONTENTS as an additional condition, we'd later create an anonymous render block in RenderBlock::addChildIgnoringContinuation: if (newChild->isInline()) { // No suitable existing anonymous box - create a new one. RenderBlock* newBox = createAnonymousBlock(); RenderBox::addChild(newBox, beforeChild); newBox->addChild(newChild); return; } So we really need to move this logic to when a RenderObject is created instead of at the time of style resolution.
Antti Koivisto
Comment 5 2016-11-14 13:58:26 PST
Antti Koivisto
Comment 6 2016-11-14 14:20:41 PST
Build Bot
Comment 7 2016-11-14 15:26:17 PST
Comment on attachment 294742 [details] patch Attachment 294742 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2516046 New failing tests: css3/flexbox/flexitem.html fast/css-grid-layout/grid-item-display.html
Build Bot
Comment 8 2016-11-14 15:26:21 PST
Created attachment 294757 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 9 2016-11-14 15:31:59 PST
Comment on attachment 294742 [details] patch Attachment 294742 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2516067 New failing tests: css3/flexbox/flexitem.html fast/css-grid-layout/grid-item-display.html
Build Bot
Comment 10 2016-11-14 15:32:04 PST
Created attachment 294758 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 11 2016-11-14 15:37:21 PST
Comment on attachment 294742 [details] patch Attachment 294742 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2516035 New failing tests: fast/css-grid-layout/grid-item-display.html fast/shadow-dom/css-scoping-slot-flex.html
Build Bot
Comment 12 2016-11-14 15:37:26 PST
Created attachment 294763 [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.11.6
Build Bot
Comment 13 2016-11-14 15:42:05 PST
Comment on attachment 294742 [details] patch Attachment 294742 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2516108 New failing tests: css3/flexbox/flexitem.html fast/css-grid-layout/grid-item-display.html
Build Bot
Comment 14 2016-11-14 15:42:10 PST
Created attachment 294764 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Antti Koivisto
Comment 15 2016-11-14 16:03:54 PST
Ryosuke Niwa
Comment 16 2016-11-14 16:41:22 PST
Comment on attachment 294769 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=294769&action=review > LayoutTests/fast/shadow-dom/css-scoping-slot-flex.html:105 > + Could you add a test case where a slot to assigned to another slot as well as a case where you override display type of slot so that conditions in TreeResolver::parentBoxStyle() would be tested?
Antti Koivisto
Comment 17 2016-11-15 10:49:04 PST
Created attachment 294850 [details] patch for landing
WebKit Commit Bot
Comment 18 2016-11-15 11:53:29 PST
Comment on attachment 294850 [details] patch for landing Clearing flags on attachment: 294850 Committed r208743: <http://trac.webkit.org/changeset/208743>
WebKit Commit Bot
Comment 19 2016-11-15 11:53:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.