Closed Bug 217000 Opened 21 years ago Closed 21 years ago

calling document.open from a javascript: src= mixed with frames causes silent failures

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.6alpha

People

(Reporter: blizzard, Assigned: jst)

References

Details

(Whiteboard: [HAVE FIX])

Attachments

(2 files)

This is from jst's email:


While loading the testcase you sent me, we end up calling document.open from a
javascript: URL in a src of a frame element, and the call to document.open is
made before the document's presshell is created (i.e. before we start displaying
it). The current code can't quite cope with that, so I made some changes to make
it deal (see attached patch). Not sure how safe this is yet, but it makes your
testcase load, at least mostly. The progress bar still doesn't stop, and I don't
know why yet. It could be due to a problem with javascript: URL's again, or it
could be something else.

At least the silent script error is fixed by the attached patch, but I don't
know if that's enough. Could you apply the attached patch and try it in the
reall webapp and report back what you get with this patch?
Attached patch patchSplinter Review
Patch from jst.
jst, do you have any further thoughts about this patch?
heloooo
Comment on attachment 130230 [details] [diff] [review]
patch

AFAICT, this is the right thing to do. Requesting reviews.
Attachment #130230 - Flags: superreview?(peterv)
Attachment #130230 - Flags: review?(caillon)
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: PC → All
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.6alpha
Comment on attachment 130230 [details] [diff] [review]
patch

>Index: content/html/document/src/nsHTMLDocument.cpp
>===================================================================

>-  nsCOMPtr<nsIDocShell> docshell;
>+  nsCOMPtr<nsIDocShell> docshell = do_QueryReferent(mDocumentContainer);
> 
>   // Stop current loads targeted at the window this document is in.
>-  if (mScriptGlobalObject) {
>-    mScriptGlobalObject->GetDocShell(getter_AddRefs(docshell));
>-
>-    if (docshell) {
>-      nsCOMPtr<nsIWebNavigation> webnav(do_QueryInterface(docshell));
>-      webnav->Stop(nsIWebNavigation::STOP_NETWORK);
>-    }
>+  if (docshell) {
>+    nsCOMPtr<nsIWebNavigation> webnav(do_QueryInterface(docshell));
>+    webnav->Stop(nsIWebNavigation::STOP_NETWORK);
>   }

After talking this over we agreed that we don't want this change.
Attachment #130230 - Flags: superreview?(peterv) → superreview+
So what does that mean, exactly?  That the patch is bad?  Or that you just don't
need part of it?
Hehe, well, the part of the change we don't want is the part that makes us call
Stop() on the nsIWebNavigation if we do have a docshell (in mDocumentContainer),
but no mScriptGlobal. So the part in comment #5 needs to be rewritten to never
get the docshell from the script global, but only call Stop() on the
nsIWebNavigation if mScriptGlobal is non-null.
Comment on attachment 130230 [details] [diff] [review]
patch

Thought I had already marked this with r=caillon, but bugzilla must not have
picked it up.
Attachment #130230 - Flags: review?(caillon) → review+
Fix checked in on the trunk. If this is needed on a branch, I'd like to see it
sit on the trunk for a while before landing it on a branch.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Has there been a bug created to deal with why the progress bar doesn't stop (and
throbber from the test case I've been using for bug 166328)?
*** Bug 166328 has been marked as a duplicate of this bug. ***
I suspect the outstanding issue is bug 143398 or related to it.
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: