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: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Myles C. Maxfield
2014-04-09 21:06:44 PDT
Created attachment 229022 [details]
Patch
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. Created attachment 229063 [details]
Patch
This can be tested automatically once https://bugs.webkit.org/show_bug.cgi?id=131502 gets fixed. 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. Created attachment 229081 [details]
Patch
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. Comment on attachment 229081 [details]
Patch
r=me assuming the test is fixed.
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(). (In reply to comment #11) > http://trac.webkit.org/changeset/167151 Build fix in r167160: <http://trac.webkit.org/changeset/167160> *** Bug 131296 has been marked as a duplicate of this bug. *** *** Bug 131590 has been marked as a duplicate of this bug. *** |