Bug 131475

Summary:

Autocorrection causes ASSERT when replacing alternative string

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

New Bugs

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ddkilzer, dino, enrica, jonlee, mark.lam, phiw2, rniwa, simon.fraser, skinnybill, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	rniwa: review+

Myles C. Maxfield

Reported 2014-04-09 21:06:44 PDT

Autocorrection causes ASSERT when replacing alternative string

Attachments
Patch (6.70 KB, patch) 2014-04-09 21:12 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (4.74 KB, patch) 2014-04-10 12:00 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (4.84 KB, patch) 2014-04-10 15:05 PDT, Myles C. Maxfield	rniwa: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Myles C. Maxfield

Comment 1 2014-04-09 21:12:19 PDT

Created attachment 229022 [details] Patch

Myles C. Maxfield

Comment 2 2014-04-09 21:17:54 PDT

<rdar://problem/16284891>

Myles C. Maxfield

Comment 3 2014-04-09 21:18:45 PDT

This test is currently broken; I'm not sure how to bring up the autocorrect popup. I'll figure that out tomorrow and then update the test.

Myles C. Maxfield

Comment 4 2014-04-10 12:00:31 PDT

Created attachment 229063 [details] Patch

Myles C. Maxfield

Comment 5 2014-04-10 12:01:10 PDT

This can be tested automatically once https://bugs.webkit.org/show_bug.cgi?id=131502 gets fixed.

Ryosuke Niwa

Comment 6 2014-04-10 13:08:59 PDT

Comment on attachment 229063 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229063&action=review > Source/WebCore/editing/AlternativeTextController.cpp:278 > - int paragraphStartIndex = TextIterator::rangeLength(Range::create(*m_frame.document(), m_frame.document(), 0, paragraphRangeContainingCorrection.get()->startContainer(), paragraphRangeContainingCorrection.get()->startOffset()).get()); > + // Find the root container for the node, so we can create a range from that point > + Node* rootNode = paragraphRangeContainingCorrection.get()->startContainer(); > + ContainerNode* rootContainer = isContainerNode(*rootNode) ? toContainerNode(rootNode) : rootNode->parentNode(); > + while (rootContainer->parentNode()) > + rootContainer = rootContainer->parentNode(); A better way to do this will be obtaining the tree scope's rootNode. > ManualTests/autocorrection/autocorrection-accept-crash.html:4 > +<head> > +</head> We don't need head. > ManualTests/autocorrection/autocorrection-accept-crash.html:12 > +<p> > +<ol> > +<li>Type "word loadp" in the first box.</li> > +<li>When the suggestion popup appears, click in the second box.</li> > +</ol> > +The test is successful if there is no crash. > +</p> p can't contain ol. Also, please describe what we're testing. > ManualTests/autocorrection/autocorrection-accept-crash.html:18 > +<p> > +<input id="t" type="text" spellCheck="true"> > +</p> > +<p> > +<textarea id="a"></textarea> > +</p> I don't think we need all these p's.

Myles C. Maxfield

Comment 7 2014-04-10 15:05:22 PDT

Created attachment 229081 [details] Patch

Ryosuke Niwa

Comment 8 2014-04-11 13:59:01 PDT

Comment on attachment 229081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229081&action=review > ManualTests/autocorrection/autocorrection-accept-crash.html:18 > +<ol> > +<li>Type "word loadp" in the first box.</li> > +<li>When the suggestion popup appears, click in the second box.</li> > +</ol> This instruction should come first.

Ryosuke Niwa

Comment 9 2014-04-11 13:59:28 PDT

Comment on attachment 229081 [details] Patch r=me assuming the test is fixed.

Ryosuke Niwa

Comment 10 2014-04-11 14:00:11 PDT

Comment on attachment 229081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229081&action=review > Source/WebCore/editing/AlternativeTextController.cpp:275 > + int paragraphStartIndex = TextIterator::rangeLength(Range::create(*m_frame.document(), &rootNode, 0, paragraphRangeContainingCorrection.get()->startContainer(), paragraphRangeContainingCorrection.get()->startOffset()).get()); You can use rootNode.document().

Myles C. Maxfield

Comment 11 2014-04-11 14:37:40 PDT

http://trac.webkit.org/changeset/167151

David Kilzer (:ddkilzer)

Comment 12 2014-04-11 15:16:30 PDT

(In reply to comment #11) > http://trac.webkit.org/changeset/167151 Build fix in r167160: <http://trac.webkit.org/changeset/167160>

Alexey Proskuryakov

Comment 13 2014-04-14 09:59:57 PDT

*** Bug 131296 has been marked as a duplicate of this bug. ***

Alexey Proskuryakov

Comment 14 2014-04-14 10:00:09 PDT

*** Bug 131590 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.