373008 - [spatial] [@ nsSpatialNavigation::getContentInDirection - nsSpatialNavigation::handleMove]

Status: RESOLVED INCOMPLETE

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[timeless]

nsSpatialNavigation::handleMove
doesn't check to verify that getFrameForContent doesn't fail or return null (and it can).

it then calls isArea(focusedContent) which will say false for a null pointer.

and then it calls makeRectRelativeToGlobalView(focusedContent) which doesn't change the pointer.

lastly it calls getContentInDirection which crashes.

(gdb) bt
#0  0x4210e3c0 in nsSpatialNavigation::getContentInDirection (this=0x393d00, aDirection=1, aPresContext=0x7f8320, aFocusedRect=@0xbe9a9550,

    aFocusedFrame=0x0, aIsAREA=0, aFocusDocuments=1108403992, aContent=0x0) at nsIFrame.h:530
#1  0x4210e718 in nsSpatialNavigation::handleMove (this=0x393d00, direction=1)
    at mozilla/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:601

#2  0x4210dc80 in nsSpatialNavigation::KeyPress (this=0x393d00, aEvent=0x1430570)
    at mozilla/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:238

#3  0x417da92c in nsEventListenerManager::RemoveEventListenerByType (this=0x1, aListener=0xbe9a9658, aType=@0xfd55c0, aFlags=9932424, aEvtGrp=0x0)
    at nsIAtom.h:493

please don't ask me for steps to reproduce, i was either using google calendar or google mail. if i was using google calendar, i might have used a data:text/html,<frameset rows="300,*"><frame src="...calendar..."><frameset>
url in order to restrict my window.

Comment 1

[Kevin Zhang]

Attachment: add checking for getFrameForContent's return value, in handleMove

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 257667 [details] [diff] [review]
add checking for getFrameForContent's return value, in handleMove

Sorry, I can't really review this code... ask whoever owns it?

Comment 3

[timeless]

i don't think it's particularly strongly owned, the question is mostly about what frames mean and such, and would this be a remotely reasonable behavior.

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 257667 [details] [diff] [review]
add checking for getFrameForContent's return value, in handleMove

or test against |focusedFrame|.

Comment 5

[Mats Palmgren (inactive)]

extensions/spatialnavigation/src/nsSpatialNavigation.cpp doesn't exist anymore.
It seems to be all JS now: toolkit/modules/SpatialNavigation.jsm