[webkit-reviews] review denied: [Bug 74585] Crash when navigating with arrow key into empty anchor block with padding : [Attachment 120810] Patch take three

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Daniel Jalkut
<jalkut at red-sweater.com>'s request for review:
Bug 74585: Crash when navigating with arrow key into empty anchor block with
padding
https://bugs.webkit.org/show_bug.cgi?id=74585

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=120810&action=review


> ManualTests/crash-on-arrow-into-empty-anchor.html:9
> +<li>Press the up or down arrow key to attempt to enter the red
rectangle.</li>

You should be able to automate this using getSelection().modify. r- for this.

> Source/WebCore/ChangeLog:8
> +	   Return 0 from closestLeafChildForLogicalLeftPosition instead of
crashing when a non-leaf box with no children is being asked for its leaf
children. Adjust logic for callers in previousLinePosition and nextLinePosition
to detect 0 response and treat the box as non-navigable.

This line is way too long. Please wrap line as needed. See other entries for
example.

> Source/WebCore/editing/visible_units.cpp:716
> +	   InlineBox* leafChild = root->closestLeafChildForPoint(pointInLine,
isEditablePosition(p));
> +	   if (leafChild) {

It's odd that we can get null here. What is box in line 702 then? Is it a root
inline box? We should probably check that root box as at least line leaf in
line 700 immediately after pos.getInlineBoxAndOffset(DOWNSTREAM, box,
ignoredCaretOffset); and fall back to return VisiblePosition(pos, DOWNSTREAM);
because that's the code path we normally use for an empty block.