Bug 146038 – Position::findParent() should take a reference

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Jon Honeycutt]

Position::findParent() should take a reference.

[Jon Honeycutt]

Created attachment 254982 [details]
Patch

[Darin Adler]

Comment on attachment 254982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254982&action=review

> Source/WebCore/dom/Position.cpp:165
> +        return findParent(*m_anchorNode.get());

No need for .get() here.

> Source/WebCore/dom/Position.cpp:225
> +        if (findParent(*m_anchorNode.get()) && (editingIgnoresContent(m_anchorNode.get()) || isRenderedTable(m_anchorNode.get())))

No need for .get() in the *m_anchorNode part.

> Source/WebCore/dom/Position.cpp:314
> +        if (!node)
> +            return *this;

Looks like this fixes a bug. Can we make a test that shows this was broken before?

> Source/WebCore/dom/Position.cpp:367
> +        if (!node)
> +            return *this;

Looks like this fixes a bug. Can we make a test that shows this was broken before?

[Jon Honeycutt]

(In reply to comment #2)
> Comment on attachment 254982 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254982&action=review
> 
> > Source/WebCore/dom/Position.cpp:165
> > +        return findParent(*m_anchorNode.get());
> 
> No need for .get() here.
> 
> > Source/WebCore/dom/Position.cpp:225
> > +        if (findParent(*m_anchorNode.get()) && (editingIgnoresContent(m_anchorNode.get()) || isRenderedTable(m_anchorNode.get())))
> 
> No need for .get() in the *m_anchorNode part.

Fixed.

> 
> > Source/WebCore/dom/Position.cpp:314
> > +        if (!node)
> > +            return *this;
> 
> Looks like this fixes a bug. Can we make a test that shows this was broken
> before?
> 
> > Source/WebCore/dom/Position.cpp:367
> > +        if (!node)
> > +            return *this;
> 
> Looks like this fixes a bug. Can we make a test that shows this was broken
> before?

I couldn’t find a way to make this crash. I looked for existing bug reports and found <rdar://problem/21026135>, but I was unable to reproduce a crash given the repro steps in the bug.

Thanks for the review!

[Jon Honeycutt]

Committed r185682: <http://trac.webkit.org/changeset/185682>