[Webkit-unassigned] [Bug 66681] Need API for getting surrounding text from webkit in chromium
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Aug 30 13:49:10 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=66681
Ryosuke Niwa <rniwa at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #105686|review? |review-
Flag| |
--- Comment #16 from Ryosuke Niwa <rniwa at webkit.org> 2011-08-30 13:49:09 PST ---
(From update of attachment 105686)
View in context: https://bugs.webkit.org/attachment.cgi?id=105686&action=review
> Source/WebCore/editing/FrameSelection.cpp:1887
> +bool FrameSelection::getSurroundingText(String& text, size_t& focus, size_t& anchor) const
It seems like the function name should reflect the fact it only obtains text in the content editable area. Also, I'd expect the return value of a function named get...Text to be a string.
> Source/WebCore/editing/FrameSelection.cpp:1893
> + if (!isContentEditable())
> + return false;
> +
> + Element* element = rootEditableElement();
> + if (!element)
This check is redundant. If there's no root editable element, then it's surely not editable.
> Source/WebCore/editing/FrameSelection.cpp:1900
> + RefPtr<Range> testRange = Range::create(
> + element->document(), element, 0,
> + base().containerNode(), base().offsetInContainerNode());
You should call computeOffsetInContainerNode instead. Otherwise this will hit an assertion. r- because of this.
> Source/WebCore/editing/FrameSelection.cpp:1905
> + testRange->setEnd(extent().containerNode(),
> + extent().offsetInContainerNode(), ec);
Why don't you just do setEnd(extent(), ec) ?
> Source/WebCore/editing/FrameSelection.cpp:1908
> + // FIXME truncate the text into reansonable size
Nit: need a colon after FIXME.
> Source/WebCore/editing/FrameSelection.h:246
> + bool getSurroundingText(String&, size_t&, size_t&) const;
You probably need to say what these size_t's are.
> Source/WebKit/chromium/src/WebViewImpl.cpp:1513
> + String tmp;
Let's not use such an ambiguous shorthanded name. We can rename text to webText or passedText. Alternatively we can rename tmp to coreText.
--
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