392060 - snav crashed with dynamic/multant DOM contents

Status: RESOLVED WORKSFORME

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

Description

[Antonio Gomes (tonikitoo)]

PRECONDITIONS: 
firefox 2 built with 'spatialnavigation' extension enabled.

mozconfig:

_________________________________________
mk_add_options MOZ_CO_PROJECT=browser
ac_add_options --enable-application=browser

ac_add_options --enable-extensions=spatialnavigation
ac_add_options --disable-airbag
_________________________________________


STEPS LEADING TO PROBLEM: 
1. go to 'www.aol.com'
2. Using snav (or not), focus highlight any link in the "featured stories" box near the center/top of the page (see attachment).
   2.1 Note that the contents of this box rotates over time. Allow it to rotate to another content item.
3. Press a navigation key

EXPECTED OUTCOME:
browser does not crash

ACTUAL OUTCOME:
FF2 crashes

FREQUENCY OF OCCURRENCE: 
always

Comment 1

[Antonio Gomes (tonikitoo)]

Attachment: frame where crash happens

well ... see frame where crash occurs.

btw, firefox 2 (and also 3) renders this webpage badly, so I sshot'ed it with opera :/

Comment 2

[Antonio Gomes (tonikitoo)]

Attachment: patch adds some needed NULL point checks into snav

Comment 3

[Antonio Gomes (tonikitoo)]

doug could you check this in ?

Comment 4

[Doug Turner (:dougt)]

sure, i can.  The patch above is for the 1.8 branch.  Can you put together a trunk patch really quick and I will land both at the same time.

Comment 5

[Antonio Gomes (tonikitoo)]

Attachment: patch adds some needed NULL point checks into snav source (v2)

I added some further NULL point checks

Comment 6

[Antonio Gomes (tonikitoo)]

Attachment: patch adds some needed NULL point checks into snav source (v3)

removed wrongly "tab"

Comment 7

[Antonio Gomes (tonikitoo)]

Attachment: patch adds some needed NULL point checks into snav source (trunk - ff3)

snav still fails to build in FF3/trunk (with or without this patch applied) due to bug 347731, however the changes proposed in this patch do not affect it at all, and it can be committed.

Comment 8

[Doug Turner (:dougt)]

I am not sure it is a good idea to land anything on something that can't compile and sort of don't want to have the code diverge.  

Let me go over and comment on 347731.

Comment 9

[Doug Turner (:dougt)]

mass reassigning to nobody.

Comment 10

[Doug Turner (:dougt)]

Comment on attachment 277238 [details] [diff] [review]
patch adds some needed NULL point checks into snav source (v3)

i no longer support extensions/spatialnavigation.  I rewrote this feature set in js and that is what I am currently using.

Comment 11

[Jesse Ruderman]

The code has been rewritten in JS.  The C++ version is no longer in the repo.