[Webkit-unassigned] [Bug 16279] VisiblePosition::canonicalPosition can wrongly return a Position object with a null m_node

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=16279


Ojan Vafai <ojan at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tony at chromium.org




--- Comment #7 from Ojan Vafai <ojan at chromium.org>  2010-06-16 15:11:02 PST ---
Bug 30116 has a couple cases where returning null causes crashes. In that case, we added null-checks higher up. But those aren't really a complete fix. They were just a "not crashing is better than crashing" solution.

A quick scan of the deepEquivalent calls in WebCore shows a ton of places where we don't null-check and probably should. For example, http://trac.webkit.org/browser/trunk/WebCore/editing/CompositeEditCommand.cpp#L661. We either need to change the calling code throughout the codebase to null-check or we need to assert that canonicalPosition only returns a null Position if the passed in Position was null.

Maybe as an interim step, we can add ASSERTs to canonicalPosition anywhere we might return null. That way, we can find some cases where we hit this and that will help decide whether we should be returning non-null, or null-checking higher up. If we do this, the if-statement modified in this patch would probably want an ASSERT_NOT_REACHED and the final return value of canconicalPosition would need to assert "next" is not null.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.