448860 - Crash when click imagemap [@ nsImageMap::ChangeFocus(nsIDOMEvent*, int) ]

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Hideo Oshima]

Steps to reproduce:
1. Open http://www.sgm.co.jp/tempo/index.html
2. Click "関東" on the map. It is lower right corner on the map.

Actual result:
Firefox crashes.

Crash Report is
http://crash-stats.mozilla.com/report/pending/30318394-607c-11dd-b0ec-001a4bd43ef6

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a2pre) Gecko/2008080202 Minefield/3.1a2pre

Comment 1

[Mats Palmgren (inactive)]

Attachment: stack

The synchronous Invalidate() destroys the image frame, which clears the
array of areas we're looping...
http://hg.mozilla.org/mozilla-central/index.cgi/annotate/49c8615d5d46e5e2b539844afb13ef8ae6f98609/layout/generic/nsImageMap.cpp#l986

Comment 2

[Mats Palmgren (inactive)]

Bug also occurs in 3.0.2pre.  Patch coming up...

Comment 3

[Mats Palmgren (inactive)]

Attachment: mochitest.diff

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1 (cvs diff -w)

This patch is against CVS trunk.  The patch is the same for mozilla-central
but "hg diff -w" wasn't able to eliminate the white space difference
like cvs can, so it's more convenient for review.

Changes:

-        if (areaContent) {

It's redundant.

-                    imgFrame->Invalidate(dmgRect, PR_TRUE);
+                  imgFrame->Invalidate(dmgRect, PR_FALSE);

This is the crash fix.  I don't see any reason it needs to be synchronous.

+          break;

No need to continue the loop if we found the <area> we're looking for.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is there a reason to make the test a mochitest instead of a crashtest?

Comment 6

[Mats Palmgren (inactive)]

The only way I know of to put focus on a <area> is to synthesize a TAB
key event and for that I used EventUtils.js which requires privileges.
(bug 448676 will enable that for reftest/crashtest)

Comment 7

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/index.cgi/rev/aab10d0abbe3
http://hg.mozilla.org/mozilla-central/index.cgi/rev/1dfed577847d

-> FIXED

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 332066 [details] [diff] [review]
Patch rev. 1 (cvs diff -w)

Low risk crash fix.

Comment 9

[Samuel Sidler (old account; do not CC)]

Comment on attachment 332066 [details] [diff] [review]
Patch rev. 1 (cvs diff -w)

Approved for 1.9.0.2. Please land in CVS. a=ss

Be sure to land the mochitest as well.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into 1.9.0 branch with test.

Comment 11

[Marcia Knous [:marcia]]

verified fixed on the 1.9.0 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2pre) Gecko/2008082004 GranParadiso/3.0.2pre. I verified by using the STR in Comment 0.

verified on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080820020636 Minefield/3.1a2pre.