[Webkit-unassigned] [Bug 36359] Double clicking page's last editable inline element doesn't select a word.

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from MORITA Hajime <morrita at google.com>  2010-05-27 22:05:29 PST ---
Hi Ojan, thank you for reviewing.
Although It might be better to take a look from others,
I updated the patch to follow last feedback anyway.

> Does the SimplifiedBackwardsTextIterator on TextIterator.cpp line 195 need TextIteratorEndsAtEditingBoundary as well?
It doesn't need such check because m_pastStartNode is used to stop m_node going next,
and m_node is already guarded by editing boundary in setCurrentNode().
The point is that previousInPostOrderCrossingShadowBoundaries() computes 
another kind of boundary what we should care other than editing-boundary.

> 
> WebCore/editing/TextIterator.cpp:1152
>  +       if (!node)
> Indentation is off.
Fixed

> 
> WebCore/editing/TextIterator.cpp:1154
>  +      if (!m_node)
> I didn't quite mean that you'd split up the null-checks here. 
Fixed to move it out of the method, and use it in the ctor.

> WebCore/editing/TextIterator.h:187
>  +      bool willCrossEditingBoundary(Node* node) const;
> Nit: I prefer the name crossesEditingBoundary
Fixed to rename crossesEditingBoundary().

> 
> WebCore/dom/Position.h:145
>  +      Node* ascentEditingBoundary() const;
> This should probably be placed next to atEditingBoundary in the header file as well.
Fixed.

> 
> WebCore/dom/Position.cpp:335
>  +      if (!m_anchorNode->document())
> Just to clarify, I didn't mean that each null-check had to go on it's own line. It would be fine to combine 333 and 335 into one if-statement.
Fixed to combine them.

> 
> WebCore/dom/Position.cpp:331
>  +  Node* Position::ascentEditingBoundary() const
> How about parentEditingBoundary? I understand what you mean by "ascent", but I don't think I would get it from calling code without looking at this function to figure out what it does.
Agreed, and fixed to rename parentEditingBoundary().

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