[Webkit-unassigned] [Bug 36359] Double clicking page's last editable inline element doesn't select a word.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 27 13:40:36 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=36359
Ojan Vafai <ojan at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #57210|review? |review-
Flag| |
--- Comment #24 from Ojan Vafai <ojan at chromium.org> 2010-05-27 13:40:34 PST ---
(From update of attachment 57210)
I'd like darin or mitz to comment since I'm not as familiar with this code.
Just a few nits left from me.
Does the SimplifiedBackwardsTextIterator on TextIterator.cpp line 195 need TextIteratorEndsAtEditingBoundary as well?
WebCore/editing/TextIterator.cpp:1152
+ if (!node)
Indentation is off.
WebCore/editing/TextIterator.cpp:1154
+ if (!m_node)
I didn't quite mean that you'd split up the null-checks here. What I meant was that if the m_node null-check were part of the if-statement on line 1156 instead, then you could use setCurrentNode for the initial setting of m_node in the constructor. Then, the only places where m_node is set would be in setCurrentNode and clearCurrentNode. So, specifically, something like this:
bool SimplifiedBackwardsTextIterator::setCurrentNode(Node* node)
{
if (!node)
return false;
if (m_node && m_behavior == TextIteratorEndsAtEditingBoundary && willCrossEditingBoundary(node))
return false;
m_node = node;
return true;
}
WebCore/editing/TextIterator.h:187
+ bool willCrossEditingBoundary(Node* node) const;
Nit: I prefer the name crossesEditingBoundary
WebCore/dom/Position.h:145
+ Node* ascentEditingBoundary() const;
This should probably be placed next to atEditingBoundary in the header file as well.
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.
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.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list