[Webkit-unassigned] [Bug 61344] --webkit-visual-word does not work in multi-line

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61344


Xiaomei Ji <xji at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #100589|                            |review?
               Flag|                            |




--- Comment #11 from Xiaomei Ji <xji at chromium.org>  2011-07-12 16:48:12 PST ---
Created an attachment (id=100589)
 --> (https://bugs.webkit.org/attachment.cgi?id=100589&action=review)
patch w/ layout test

Thanks for the review!

(In reply to comment #8)
> (From update of attachment 96341 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96341&action=review
> 
> Great!  New patch looks much better.  But it'll be nice if rendering experts like mitz or dhyatt could give us some feedback.
> 
> > Source/WebCore/editing/htmlediting.h:144
> > +inline Position createPosition(Node* node, int offset)
> 
> This function name sounds too general.  How about createPosiitonAvoidingIgnoredNode?

done.

> 
> > Source/WebCore/editing/visible_units.cpp:1506
> > +        RenderObject* previousBlockRenderer = renderer->previousSibling();
> > +        while (previousBlockRenderer) {
> > +            const RenderBlock* blockWithLineBoxes = lastChildBlockWithLineBoxes(toRenderBlock(previousBlockRenderer));
> > +            if (blockWithLineBoxes)
> > +                return blockWithLineBoxes->lastRootBox();
> > +
> > +            previousBlockRenderer = previousBlockRenderer->previousSibling();
> > +        }
> 
> It seems like this loop is almost identical to the one in lastChildBlockWithLineBoxes. Can we combine these two loops?

Thanks for the code snippet. Done.

> 
> > Source/WebCore/editing/visible_units.cpp:1564
> > +    const InlineFlowBox* leftLineBox = (blockDirection == LTR) ? rootBox->prevLineBox() :
> > +                                                                 rootBox->nextLineBox();
> 
> I wonder it makes a sense to add a helper function on InlineTextBox.

no change per hyatt's feedback.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.