Mozilla crashes after clicking "SKYPASS" at Korean Air site

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: v.kazmirenko, Assigned: tor)

Tracking

({crash, testcase, verified1.7})

Trunk
x86
All
crash, testcase, verified1.7
Points:
---
Bug Flags:
blocking1.7 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 5 obsolete attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

Open http://www.koreanair.com/
At top of page there is link "SKYPASS". Click it. After opening this page
Mozilla crashes.


Reproducible: Always
Steps to Reproduce:
1.
2.
3.
(Reporter)

Updated

15 years ago
Summary: Mozilla crashes after clicking "SKYPASS" and opening page (URL is provided above) → Mozilla crashes after clicking "SKYPASS" at Korean Air site

Comment 1

15 years ago
confirming crash FF 20040302 Win2k, I get a popup R0625, Pure Virtual Call.
Keywords: crash, stackwanted

Comment 2

15 years ago
Confirming "R0625, Pure Virtual Call".
This is on Gecko/20040113, Windows 2000.

However, the behaviour isn't very reproductible here. The amount of time before
the crash seems quit unconsistent.

Comment 3

15 years ago
Confirming. Could be a dupe of 236196 but it seems to be slightly different.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

15 years ago
Also crashes on linux:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040227 Firefox/0.8.0+

Comment 5

15 years ago
Target All platforms based on #4
OS: Windows XP → All

Comment 7

15 years ago
Bug occurs in 2003-09-03-05 trunk Linux, which is the oldest nightly build
I could find.  Bug does not occur in Mozilla 1.4 on Linux.
Assignee: general → jdunn
Component: Browser-General → ImageLib

Comment 8

15 years ago
Created attachment 142886 [details]
testcase

load the testcase.  reload a few times.  crash
tested trunk build 2004030308

<img src="javascript:SKchoosePhoto()" name="SKbnrPhoto">

SKchoosePhoto is:
  document.SKbnrPhoto.src="resource:///res/samples/Anieyes.gif";

Comment 9

15 years ago
regrsession between linux trunk builds 2003071005 and 2003071122
bug 70030 or bug 212133?
I'll try backing stuff out.
Keywords: stackwanted → regression, testcase

Comment 10

15 years ago
further testing just shows that bug 212133 just got the DOM working used in the
testcase, so no regression.
Keywords: regression

Comment 11

15 years ago
I can't reproduce it on linux with
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040122 Epiphany/1.0.7
and
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040301 Firefox/0.8.0+

(Neither the original page nor the testcase...)
*** Bug 236420 has been marked as a duplicate of this bug. ***

Comment 13

15 years ago
*** Bug 236196 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Assignee: jdunn → tor
(Assignee)

Comment 14

15 years ago
Created attachment 143546 [details] [diff] [review]
prevent img src collision

What appears to be happening here is that the src="javascript:..."
load comes in first and kicks off a load.  Deeper down we notice that
it's javascript and run it, which kicks off another load on the same
image, but now with the real URI.  But since the previous load is
still ongoing we pick the wrong request to use (current instead of
pending), which gets overwritten when processing returns to the first
load.
(Assignee)

Comment 15

15 years ago
Created attachment 143547 [details] [diff] [review]
prevent img src collision

Ok, let's attach the patch and not the testcase this time...
Attachment #143546 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #143547 - Flags: review?(pavlov)
(Assignee)

Updated

15 years ago
Attachment #143547 - Flags: review?(pavlov)
(Assignee)

Comment 16

15 years ago
Created attachment 143722 [details] [diff] [review]
modify both places with test
Attachment #143547 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #143722 - Flags: review?(pavlov)
(Assignee)

Updated

15 years ago
Flags: blocking1.7?

Updated

15 years ago
Attachment #143722 - Flags: review?(pavlov) → review+
(Assignee)

Updated

15 years ago
Attachment #143722 - Flags: superreview?(bzbarsky)
Hmm.. so why is this actually crashing?  Are we double-releasing an
imgRequestProxy or something?

With that patch, won't

function foo() {
  img.src = "javascript:bar()";
}

function bar() {
  img.src = "http://whatever";
}

<img src="javascript:foo()">

still crash?  This time because mPendingRequest is clobbered?
Oh, and no matter what that should be a PRPackedBool and live with the other
PRPackedBools.
(Assignee)

Comment 19

15 years ago
Created attachment 145215 [details]
double-indirect testcase
(Assignee)

Comment 20

15 years ago
Comment on attachment 143722 [details] [diff] [review]
modify both places with test

Yes, the double-indirect thing kills it.
Attachment #143722 - Attachment is obsolete: true
Attachment #143722 - Flags: superreview?(bzbarsky)

Comment 21

15 years ago
*** Bug 235843 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 22

15 years ago
Created attachment 145337 [details] [diff] [review]
different fix - need to discuss with pavlov

Comment 23

15 years ago
*** Bug 240184 has been marked as a duplicate of this bug. ***

Comment 24

15 years ago
Tor, any progress here? This one seems like a good one to try to get for 1.7.
Flags: blocking1.7? → blocking1.7+

Comment 25

15 years ago
*** Bug 242334 has been marked as a duplicate of this bug. ***

Comment 26

15 years ago
Comment on attachment 145337 [details] [diff] [review]
different fix - need to discuss with pavlov

i ran with this patch and i can't find anything that breaks.. go for it
Attachment #145337 - Flags: review+
(Assignee)

Updated

15 years ago
Attachment #145337 - Flags: superreview?(darin)

Comment 27

15 years ago
Comment on attachment 145337 [details] [diff] [review]
different fix - need to discuss with pavlov

you seem to be transforming the retval of imgILoader::LoadImage into an inout
param.	is that right?	if so, shouldn't you change the IDL accordingly?
(Assignee)

Comment 28

15 years ago
Created attachment 147651 [details] [diff] [review]
add [inout] to IDL
Attachment #145337 - Attachment is obsolete: true

Comment 29

15 years ago
I think we decided on IRC that there is no point to supporting [inout] on the
result of LoadImage (that all callers anyways just want the resulting value and
never pass anything but null).  so, how about just making sure that the
implementation of LoadImage sets *_retval to nsnull before doing anything else.
 that way, the fact that CreateNewProxyForRequest treats _retval as an inout
param will be inconsequential to the caller of LoadImage or LoadImageWithChannel.

Comment 30

15 years ago
To clarify my last comment: whatever we do for LoadImage needs to be done for
LoadImageWithChannel as well.
(Assignee)

Comment 31

15 years ago
Created attachment 147664 [details] [diff] [review]
tweak per irc discussion
Attachment #147651 - Attachment is obsolete: true

Comment 32

15 years ago
Comment on attachment 147664 [details] [diff] [review]
tweak per irc discussion

sr=darin

might be good to indicate with a comment the reason why it is significant that
*_retval be nulled out.
Attachment #147664 - Flags: superreview+
(Assignee)

Comment 33

15 years ago
Checked in on trunk with comments added.  Leaving open for baking.

Comment 34

15 years ago
I noticed some strange image loading behaviour in a build that I did this morning.
My last build was on 5/3 and I didn't see the bahviour from that build.

I have a homepage with several small images on it which link to larger versions
of the images and they are all local to my hard drive. When I click on one of the
smaller images, the page area is blanked out (white) for a moment before the
image is displayed. Sometimes, this blank time is a fraction of a second and
sometimes it's as long as three seconds.

The image paints very quickly after the pause so it appears that something's going
on that's not related to image painting.

I also noticed this with the MozillaZine png at the top of the page though it
is very short there as the PNG is probably fairly small.

Someone pointed me to this checkin as a possibility.

Comment 35

15 years ago
Note: the behavior described in #34 is not limited to pages with images. On an
internal site I'm developing (which uses only stylesheets) I've noticed that
between the 20040506 zip build and the 20040505 zip build (Windows Firefox),
many pages are MUCH slower on initial load. It seems once they're cached it's fine.

In particular, the pages w/ form controls of any kind, be it text entry boxes,
or form buttons, are noticeable... you can easily see the form component being
drawn before all other page content.

Should another bug be opened about the slow rendering?

Comment 36

15 years ago
This checkin wasn't the cause. I backed it out, did a build and it's still there.
I'll file a bug for now.

Comment 37

15 years ago
Filed bug http://bugzilla.mozilla.org/show_bug.cgi?id=242856 for the other problem that is NOT related to this bug.

Updated

15 years ago
Attachment #145337 - Flags: superreview?(darin)
(Assignee)

Updated

15 years ago
Attachment #147664 - Flags: approval1.7?

Comment 38

15 years ago
Comment on attachment 147664 [details] [diff] [review]
tweak per irc discussion

a=mkaply
Attachment #147664 - Flags: approval1.7? → approval1.7+
(Assignee)

Comment 39

15 years ago
Checked in on branch.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Updated

15 years ago
Keywords: fixed1.7

Comment 40

14 years ago
Verified as fix on latest 1.7 branch Win06-24, Mac06-30 & Linux06-29 builds.
Changing keywords from fixed1.7 to verified1.7.
Leave this bug status "as is" until this bug be verified on trunk again...
Keywords: fixed1.7 → verified1.7
You need to log in before you can comment on or make changes to this bug.