RESOLVED FIXED Bug 131475
Autocorrection causes ASSERT when replacing alternative string 
https://bugs.webkit.org/show_bug.cgi?id=131475
Summary Autocorrection causes ASSERT when replacing alternative string
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
DetailsFormatted DiffDiff
Patch (4.74 KB, patch)
2014-04-10 12:00 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (4.84 KB, patch)
2014-04-10 15:05 PDT, Myles C. Maxfield 		rniwa: review+
DetailsFormatted DiffDiff
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.