[webkit-reviews] review granted: [Bug 178332] Text nodes with display:contents parent should render as if they were wrapped in an unstyled <span> : [Attachment 324001] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 17 02:12:50 PDT 2017


Ryosuke Niwa <rniwa at webkit.org> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 178332: Text nodes with display:contents parent should render as if they
were wrapped in an unstyled <span>
https://bugs.webkit.org/show_bug.cgi?id=178332

Attachment 324001: patch

https://bugs.webkit.org/attachment.cgi?id=324001&action=review




--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 324001
  --> https://bugs.webkit.org/attachment.cgi?id=324001
patch

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

> Source/WebCore/rendering/RenderElement.cpp:228
> +	   auto* textRendererWithDisplayContentsParent =
RenderText::findForDisplayContentsInlineWrapper(rendererForFirstLineStyle);

Maybe we call this findByDisplayContentsInlineWrapperCandidate instead?

> Source/WebCore/rendering/RenderText.cpp:137
> +    static NeverDestroyed<HashMap<const RenderText*, WeakPtr<RenderInline>>>
map;

It would be nice if we can make it so that nobody can possibly de-reference
RenderText* so that it's UAF-safe
but that'd require quite a bit of hash traits hackery... :(

> Source/WebCore/rendering/RenderText.cpp:1761
> +	   inlineWrapperForDisplayContentsMap().remove(this);

Can we assert that the entry did exist in the map?

> Source/WebCore/rendering/RenderText.cpp:1765
> +    inlineWrapperForDisplayContentsMap().add(this, makeWeakPtr(wrapper));

Can we assert that this is isNewEntry?

> LayoutTests/ChangeLog:10
> +2017-10-16  Antti Koivisto  <antti at apple.com>

Double change log entries.


More information about the webkit-reviews mailing list