[webkit-reviews] review denied: [Bug 10123] when CSS pseudo selectors are applied (:before and :after) the *-of-line keyboard navigation does not work : [Attachment 110843] Solved the navigation issue with rtl texts.

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Rosen Dash
<rosen.dash at motorola.com>'s request for review:
Bug 10123: when CSS pseudo selectors are applied (:before and :after) the
*-of-line keyboard navigation does not work
https://bugs.webkit.org/show_bug.cgi?id=10123

Attachment 110843: Solved the navigation issue with rtl texts.
https://bugs.webkit.org/attachment.cgi?id=110843&action=review

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


> LayoutTests/editing/selection/css-pseudo-element-hang.html:16
> +<span class="quote">content</span>

I'd also like to see a test case where there is some text on before/after the
span.

> LayoutTests/editing/selection/css-pseudo-element-hang.html:26
> +for(var i = 0; i < 9; ++i) {
> +    window.getSelection().modify('move', 'right', 'character');
> +}

Nit: Please put a space after "for" before (
Also not curly brackets around a single statement.

> Source/WebCore/editing/VisiblePosition.cpp:198
> +		       if (prevBox->prevLeafChild())
> +			   return box->isLeftToRightDirection() ?
previousVisuallyDistinctCandidate(m_deepPosition) :
nextVisuallyDistinctCandidate(m_deepPosition);

I don't think this is right. If we have mixed bidi in generated contents, then
we'll have a separate line box for each one of them. So it's not guaranteed
that prevBox->prevLeafChild() exists.