Closed Bug 149307 Opened 22 years ago Closed 22 years ago

Some popup windows do not render in PPEmbed

Categories

(Core Graveyard :: Embedding: Mac, defect)

Product:
Core Graveyard
Component:
Embedding: Mac
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla1.0.1

People

(Reporter: moconnell, Assigned: ccarlen)

References

Details

(Keywords: topembed+)

Attachments

(5 files)

Simplified test case
1.91 KB, text/html
Details
Patch for mfcembed
2.75 KB, patch
rpotts
: review+
alecf
: superreview+
Details | Diff | Splinter Review
Another test case
818 bytes, text/html
Details
Patch for docshell
776 bytes, patch
ccarlen
: review+
rpotts
: superreview+
Details | Diff | Splinter Review
patch to isolate the change to the javascript protocol handler (where it's needed)
1.27 KB, patch
adamlock
: review+
jst
: superreview+
Details | Diff | Splinter Review 
Go to www.shockwave.com. This site attempts to load a popup. However, 
in PPEmbed the popup window is created, but the contents of the window 
are never rendered.
i've seen this as well in ppembed.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: topembed
Appears to be a general embedding problem. A trunk build today of mfcEmbed shows
the problem as well. Adam, do you know anything about this?
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
Probably related to bug 142904
is anyone looking at this? should we get another owner since it's an XP bug?
Severity: critical → blocker
OS: MacOS X → All
Hardware: Macintosh → All
Blocks: 147975
Dan, can you look at this? It sounds like a problem in creating windows thru
CWindowCreator since both mfcEmbed & PPEmbed show the problem.

I'm not sure about the relation to bug 142904. Is there something which is
demanding something other than an nsIDOMWindow from the created window object?
Since danm is off on sabbatical adding rpotts as the next most likely source of
window event foo
Attached file Simplified test case — 
The shockwave site has a piece of code at the top to open up a popup advert.
I've produced a simplified test case with a formatted version of that code.

The culprit appears to be the line (that I've marked) that says "jp.blur();".
In the embedding case this causes a JS error which halts further execution
leaving a dead popup lying around. Run mfcembed with the "-console" switch to
see the error. Commenting out that line makes the script happy.

JavaScript error:
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x8
0004005 (NS_ERROR_FAILURE) [nsIDOMWindowInternal.blur]"  nsresult: "0x80004005
(
NS_ERROR_FAILURE)"  location: "JS frame ::
file:///C:/m/test/shockwave/test.html
 :: runjPop :: line 44"  data: no]
Thanks, Adam. I have a patch around somewhere for implementing focus/blur in
PPEmbed (done for bug 145827)  
Implementing nsIEmbeddingSiteWindow2::Blur() also makes the attached test case 
happy. But, it doesn't fix the actual case of going to www.shockwave.com. More
debugging...
Patch to implement blur() on mfcembed. Looking for an r/sr

After this the mfcembed console doesn't complain anymore about blur() though
the advert still does not load for some reason.
Comment on attachment 88179 [details] [diff] [review]
Patch for mfcembed

sr=alecf

which QI is failing? can we be smart and not bail if blur() isn't implemented?
Attachment #88179 - Flags: superreview+
The GlobalWindowImpl::Blur() method returns NS_ERROR_FAILURE if it can't QI for
nsIEmbeddingSiteWindow2.

http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#2377
in case anyone cares, i made chimera impl nsIEmbeddingSiteWindow2 ;)
There is another popup on this page. It wants to open a window with an url like
this:

javascript:"<HEAD><TITLE>&nbsp;</TITLE></HEAD><BODY LEFTMARGIN=0 TOPMARGIN=0
MARGINHEIGHT=0 MARGINWIDTH=0><a
href=http://servedby.advertising.com/click/site=0000088148/mnum=0000059790/genr=1/tkdt=B0P6R1T0/bnum=65081610
target=_blank><img
src=http://servedby.advertising.com/site=0000088148/mnum=0000059790/genr=1/logs=0/mdtm=1023460943/bins=1
border=0 alt='Click to learn more...'></a></BODY>"

mfcEmbed seems to be okay with simple javascript: links so I'm just looking to
see if its window.open or related or a problem with this particular url.
Attached file Another test case — 
window.open and javascript: links don't seem to mix in embedding. The test case
demonstrates what (doesn't) happen if you try window.open("javascript:2+2")
nsJSProtocolHandler is failing in AsyncOpen because the securityManager cannot
get the object principal from the newly created window. GlobalWindowImpl
normally gets it from the contained document, but since there isn't one,
everything falls apart.

Mozilla doesn't have this problem because it always loads about:blank first
before loading whatever its told to load.

Has anyone ideas how to solve this? We have a method on docshell to create an
about:blank document, so should we call that on all new windows before loading
the requested url?
Comment on attachment 88179 [details] [diff] [review]
Patch for mfcembed

r=rpotts@netscape.com
Attachment #88179 - Flags: review+
See bug 88229. It's about loading a blank document initially.
I've checked in the mfcembed patch for nsIEmbeddingSiteWindow2
This is a simple patch for docshell. It extends some existing code that tests
for the javascript: protocol in DoLoadURI to also call EnsureContentViewer.
This means there will be a document in the new window by the time the JS URL
loader is invoked so all is well.

The shockwave site is no longer showing popups (even with a fresh profile), but
the patch fixes the behaviour in the test case.

Does anyone have any other examples of busted popups to try it out on?
*** Bug 122107 has been marked as a duplicate of this bug. ***
shockwave.com is opening popups again. I've verified the patch in PPEmbed.
Conrad can you confirm / review it works for you too?
sweet! Let's try to land this today for trunk, branch, and projectx if we can!
Comment on attachment 88331 [details] [diff] [review]
Patch for docshell

r=ccarlen
Just verified with PPEmbed and www.shockwave.com. I also had the patch for bug
145827 in place which is needed for this site.
Attachment #88331 - Flags: review+
Rick, can you sr=?
I've just attached a patch which should have the same effect as adam's patch.  

Can someone test it out to verify?  I think this patch is better for the trunk
because it localizes the change to the javascript protocol handler -- which is
where the issue is.

If this patch fixes the problem, i think it is a better choice for the trunk.

-- rick

P.S.
jst told me what to type :-)

Comment on attachment 88331 [details] [diff] [review]
Patch for docshell

sr=rpotts@netscape.com

(for the branch)
Attachment #88331 - Flags: superreview+
Which branch is this for 1.0.0 or 1.0.1? I'll add edt1.0.0 for the time being
and see drivers approval
Keywords: edt1.0.0
can we get verification that rick's updated patch does the trick?
I verified that Rick's patch solves the problem as well. Any reason not to use
it for both trunk and branch?
Comment on attachment 89322 [details] [diff] [review]
patch to isolate the change to the javascript protocol handler (where it's needed)

sr=jst
Attachment #89322 - Flags: superreview+
Comment on attachment 89322 [details] [diff] [review]
patch to isolate the change to the javascript protocol handler (where it's needed)

r=adamlock

Verifying patch works in mfcEmbed. I don't mind which patch goes in. This one
makes better sense.
Attachment #89322 - Flags: review+
Fix is checked into the trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Jud, can you plus this?
landed on the chimera branch.
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1+
fixed in 1.0.1
Keywords: mozilla1.0.1+ → fixed1.0.1
No longer blocks: 147975
mdunn: pls verify this as fixed on the 1.0 branch, then replace the "fixed1.0.1"
keyword, with "verified1.0.1". thanks!
Keywords: topembed → topembed+
To Dharma for verification
QA Contact: mdunn → dsirnapalli
Verified in the branch build (2002-10-10-12-1.0) and
trunk build (2002-10-10-08-trunk) on both PPEmbed and MFCEmbed.
It works fine for both branch and trunk builds of PPEmbed.
It works fine for trunk build of MFCEmbed.
But does not work for branch build of MFCEmbed.It brings up popup window but is
blank. I tried with the first testcase. www.shockwave.com has no popup windows now.
second test case works fine on all.
Reopening the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
*** Bug 155457 has been marked as a duplicate of this bug. ***
Is this still broken on mfcembed? Who should own this bug?
Adam submitted the last patch so this seems to be in his area, if he has time.
> But does not work for branch build of MfcEmbed.

The component of this bug is "Embedding:Mac" and the summary says PPEmbed,
though the patch was XP. Reclosing.

If this is still broken in MfcEmbed, I think it should be a different bug as
it's probably a different problem.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: