[Webkit-unassigned] [Bug 232185] ASSERT(parent->element()) triggered in Styleable::fromRenderer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=232185

Tim Nguyen (:ntim) <ntim at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ntim at apple.com
 Attachment #442214|review?                     |review-
              Flags|                            |

--- Comment #2 from Tim Nguyen (:ntim) <ntim at apple.com> ---
Comment on attachment 442214
  --> https://bugs.webkit.org/attachment.cgi?id=442214
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442214&action=review

> Source/WebCore/style/Styleable.cpp:57
> +    if (!renderer.element())
> +        return std::nullopt;

This looks wrong, ::backdrop and ::marker renderers do not have any associated element, so the code below for ::backdrop & ::marker will just stop working altogether.

I'm surprised no test has caught this so far.

>From a quick look at the render tree dump, overflow-y: -webkit-paged-y on <li> seems to put the marker renderer as a child of RenderMultiColumnFlowThread instead of RenderListItem.

I'm glad the assert caught this, because it shows a real bug in the code. We should either:

* In the `case PseudoId::Marker:` branch of this function, loop through renderer ancestors until we find a `RenderListItem`, and return that.

* Simply disallow multi column flow for list items? See: https://webkit-search.igalia.com/webkit/rev/522cdac023da69b36fa895cbedea14e96f44d678/Source/WebCore/rendering/RenderBlockFlow.cpp#425-426

I wasn't able to make multicol work properly inside a list item, so maybe this is a good solution? Though not sure if there's any other feature which may wrap renderers and trigger this assert.

> LayoutTests/fast/animation/css-animation-marker-crash.html:4
> +      -webkit-mask-image: url();

maybe use a more common property like `background: green;`

> LayoutTests/fast/animation/css-animation-marker-crash.html:15
> +<li>PASS</li>

nit: PASS if this doesn't crash

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211023/2e652940/attachment.htm>