Closed Bug 448860 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Layout, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.1a2

People

(Reporter: hidenosuke, Assigned: mats)

References

()

Details

(Keywords: crash, verified1.9.0.2)

Crash Data

Attachments

(3 files)

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
Attached file 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
Assignee: nobody → mats.palmgren
Status: NEW → ASSIGNED
Assignee: mats.palmgren → nobody
Severity: normal → critical
Status: ASSIGNED → NEW
Component: General → Layout
Flags: wanted1.9.0.x?
Keywords: crash
OS: Linux → All
Product: Firefox → Core
QA Contact: general → layout
Hardware: PC → All
Assignee: nobody → mats.palmgren
Bug also occurs in 3.0.2pre.  Patch coming up...
Attached patch mochitest.diffSplinter Review
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.
Attachment #332066 - Flags: superreview?(roc)
Attachment #332066 - Flags: review?(roc)
Attachment #332066 - Flags: superreview?(roc)
Attachment #332066 - Flags: superreview+
Attachment #332066 - Flags: review?(roc)
Attachment #332066 - Flags: review+
Is there a reason to make the test a mochitest instead of a crashtest?
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)
http://hg.mozilla.org/mozilla-central/index.cgi/rev/aab10d0abbe3
http://hg.mozilla.org/mozilla-central/index.cgi/rev/1dfed577847d

-> FIXED
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
Comment on attachment 332066 [details] [diff] [review]
Patch rev. 1 (cvs diff -w)

Low risk crash fix.
Attachment #332066 - Flags: approval1.9.0.2?
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Whiteboard: [needs baking]
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.
Attachment #332066 - Flags: approval1.9.0.2? → approval1.9.0.2+
Checked into 1.9.0 branch with test.
Keywords: fixed1.9.0.2
Whiteboard: [needs baking]
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.

Status: RESOLVED → VERIFIED
Crash Signature: [@ nsImageMap::ChangeFocus(nsIDOMEvent*, int) ]
You need to log in before you can comment on or make changes to this bug.