[webkit-reviews] review denied: [Bug 62209] All pointer-events fail if text has visibility="hidden" : [Attachment 98035] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Rob Buis
<rwlbuis at gmail.com>'s request for review:
Bug 62209: All pointer-events fail if text has visibility="hidden"
https://bugs.webkit.org/show_bug.cgi?id=62209

Attachment 98035: Patch
https://bugs.webkit.org/attachment.cgi?id=98035&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98035&action=review

Almost there, sorry needs another iteration:

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:732
> +    if (isLineBreak())

AFAIK this is not possible - we have no <br> in SVG. Turn this into an
assertion.

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:736
> +    bool isVisible = (renderer()->style()->visibility() == VISIBLE);

The braces are useless.

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:744
> +		   renderer()->updateHitTestResult(result,
flipForWritingMode(pointInContainer - toSize(accumulatedOffset)));

Hm, none of the SVG code is HTML-writing-mode-aware. Our vertical text support
works differently than HTMLs newly added writing-mode support.
For now, the best is to remove writing mode related flips and tweaks.

Same for the truncation(). We don't support any truncation, so that code can be
removed.
Also locationIncludingFlipping() is probably not right, as it flippes based on
writing mode as well.