|Summary:||Support dynamic pseudo-classes on elements with display: contents|
|Product:||WebKit||Reporter:||Pavel Klimashkin <klimashkin>|
|Severity:||Normal||CC:||as.avramenko, cdumez, commit-queue, dino, ews, koivisto, rego, rniwa, simon.fraser, webkit-bug-importer, zalan|
|Version:||Safari Technology Preview|
Description Pavel Klimashkin 2018-01-14 15:56:25 PST
Pseudo-classes don't work on element with display: contents in TP 47, but they should. You can check demo here: https://codepen.io/klimashkin/pen/LepyLb?editors=1100#0 Cells in table should change style on row hover. It works correctly in Firefox and Chrome 65+, but not in Safari TP 47.
Comment 2 Antti Koivisto 2018-01-18 01:37:18 PST
:hover and :active
Comment 3 Antti Koivisto 2018-01-18 01:39:21 PST
The bug here is that hover/active style doesn't get invalidated on display:contents elements.
Comment 5 Build Bot 2018-01-23 09:21:04 PST
Comment on attachment 332029 [details] patch Attachment 332029 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6182997 New failing tests: fast/css/display-contents-hover-active.html
Comment 6 Build Bot 2018-01-23 09:21:05 PST
Created attachment 332031 [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 8 Alexey 2018-11-08 04:25:53 PST
Hi from 08.11.2018. This bug is still not fixed. Demo: http://jsfiddle.net/6208eox5/ Safari version: 11.1.2 build 13605.3.8
Comment 9 Ryosuke Niwa 2018-11-08 18:46:30 PST
Antti, is this patch still valid? Not sure why we ended up not landing this patch.
Comment 10 Antti Koivisto 2018-11-09 04:30:02 PST
(In reply to Ryosuke Niwa from comment #9) > Antti, is this patch still valid? Not sure why we ended up not landing this > patch. The patch is valid, feel free to review. It was slightly risky to land at that point due to switch to DOM based traversal and then got forgotten. Also I got sidetracked to 181680, :active was broken in general.
Comment 12 Dean Jackson 2018-11-12 10:13:00 PST
Comment on attachment 354330 [details] rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=354330&action=review > Source/WebCore/dom/Element.cpp:2920 > + if (!hasRareData()) > + return nullptr; > + auto* style = elementRareData()->computedStyle(); > + if (style && style->display() == DisplayType::Contents) Could you have if (!hasRareData() || !hasDisplayContents()) return nullptr; return elementRareData()->computedStyle(); Either way is fine with me.
Comment 13 Antti Koivisto 2018-11-12 10:34:17 PST
> Could you have > > if (!hasRareData() || !hasDisplayContents()) > return nullptr; > > return elementRareData()->computedStyle(); > > Either way is fine with me. This is slightly more efficient (in principle, meaningless in practice) since hasDisplayContents() needs to fetch the computed style from rare data too.
Comment 14 WebKit Commit Bot 2018-11-12 10:55:26 PST
Comment on attachment 354330 [details] rebased patch Clearing flags on attachment: 354330 Committed r238097: <https://trac.webkit.org/changeset/238097>
Comment 15 WebKit Commit Bot 2018-11-12 10:55:28 PST
All reviewed patches have been landed. Closing bug.
Comment 16 Pavel Klimashkin 2018-11-12 12:29:35 PST
Thanks! In what release that patch will probably land?
Comment 17 Ryosuke Niwa 2018-11-12 15:43:35 PST
(In reply to Pavel Klimashkin from comment #16) > Thanks! In what release that patch will probably land? Unfortunately, Apple does not comment on future produce release plans so we can't say. But you can usually see up-coming fixes & features in Safari Technology Preview way before it gets released to public: https://developer.apple.com/safari/technology-preview/