<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - The bounds on InteractionInformationAtPosition should be more precise"
href="https://bugs.webkit.org/show_bug.cgi?id=146468#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - The bounds on InteractionInformationAtPosition should be more precise"
href="https://bugs.webkit.org/show_bug.cgi?id=146468">bug 146468</a>
from <span class="vcard"><a class="email" href="mailto:bdakin@apple.com" title="Beth Dakin <bdakin@apple.com>"> <span class="fn">Beth Dakin</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=146468#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=255843&action=diff" name="attach_255843" title="Patch">attachment 255843</a> <a href="attachment.cgi?id=255843&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=255843&action=review">https://bugs.webkit.org/attachment.cgi?id=255843&action=review</a>
>
> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2162
> > + if (element->renderer())
> > info.touchCalloutEnabled = element->renderer()->style().touchCalloutEnabled();
>
> Seems like you can just bail early from this function if element->renderer()
> is null.
> </span >
There is more to the function that should be executed outside of an if (element) that this line of code is wrapped in. I could break; I suppose? I feel like we don't usually use breaks to break out of conditionals, but it's not covered by the style guide so maybe it's fine?
<span class="quote">> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2171
> > + RefPtr<Range> linkRange = rangeOfContents(*linkElement);
> > + Vector<FloatQuad> quads;
> > + linkRange->textQuads(quads);
> > + FloatRect linkBoundingBox;
> > + for (const auto& quad : quads)
> > + linkBoundingBox.unite(quad.enclosingBoundingBox());
> > + info.bounds = IntRect(linkBoundingBox);
>
> This is just Range::boundingRect(), but that forces layout.
> </span >
Layout should be up to date here, so that should;t be necessary. Unless we are sure it's smart enough to know things are up to date?
<span class="quote">> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2172
> > + } else if (element->renderer()) {
>
> if (RenderElement* renderer = element->renderer())...
> </span >
Fixed.
<span class="quote">> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2177
> > + info.bounds = element->renderer()->absoluteBoundingBoxRect(true);
>
> true is the default.</span >
Fixed.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>