[webkit-reviews] review denied: [Bug 36359] Double clicking page's last editable inline element causes assertion failure. : [Attachment 51390] v1; cleanup test result

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 36359: Double clicking page's last editable inline element causes assertion
failure.
https://bugs.webkit.org/show_bug.cgi?id=36359

Attachment 51390: v1; cleanup test result
https://bugs.webkit.org/attachment.cgi?id=51390&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
> @@ -175,7 +175,7 @@ static VisiblePosition nextBoundary(const
VisiblePosition& c, BoundarySearchFunc
>      Node *de = d->documentElement();
>      if (!de)
>	   return VisiblePosition();
> -    Node *boundary = n->enclosingBlockFlowElement();
> +    Node* boundary = n;
>      if (!boundary)
>	   return VisiblePosition();
>      bool isContentEditable = boundary->isContentEditable();

I'm not a good reviewer for this bug, but putting r- because I think we need
some more work to fix the bug completely.

- I think we should remove "if (!boundary)" because it was already checked.
- With this patch, "here" is selected after the double click. Without this
patch, it seems "here" isn't selected so I think you also fixed this issue and
we need a test case for this.
- With the following HTML

<div><span id="target" contentEditable="true">here</span> double click</div>

"here" isn't selected when we double-click the target node. Is this related
bug?

- (this is just a question) don't we also need to modify previousBoundary?