Closed Bug 34217 Opened 24 years ago Closed 24 years ago

javascript: URI does not execute JS, clears window instead

Categories

(Core :: Networking, defect, P1)

x86
All
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: rzach, Assigned: warrensomebody)

References

()

Details

(Keywords: top100, Whiteboard: [nsbeta2+][dogfood-]1d)

Attachments

(2 files)

Clicking on an anchor with a javascript: URI should execute the JS in the URI.
Instead, the window is cleared. 

To reproduce:
Click on javascript: link (in this bug report or in testcase).

Actual result: The viewport is cleared, the javascript: URI appears in the
location bar, the JS is not executed.

Expected result: Page stays as it is, JS code is executed (in this case, an
alert dialog should be displayed).

Seems to be a recent regression: Bug observed on Linux builds 2000.03.29.09 up
to 2000.04.01.09, but does not occur on Linux build 2000.03.28.08.   My guess is
this happend when warren checked in the Necko API changes on 03/28, so assigning
to him.
Attached file testcase
This is a must for M15, setting milestone.
Target Milestone: --- → M15
The deal is that these two lines in nsWebShell.cpp (at line 1169) are now 
checking for the js undefined value too early:

        if (rv == NS_ERROR_DOM_RETVAL_UNDEFINED) // if causing the channel 
changed the 
          return NS_OK;                        // dom and there is nothing else 
to do

This now needs to occur at the point the data of the document is loaded, 
presumably in the OnStopRequest method.
I think I'm going to need help from mscott or travis on this one.
Hmm it doesn't look like any of that code changed recently. Did the semantics
somewhere else change as far as when this return value is actually returned by
NS_OpenURI?

Assuming we need to relocate that code, one possible place is the DoContent
method on nsWebShell. Where do we get this error code on the channel warren? Can
we just get the current status of the channel and check it for
NS_ERROR_DOM_RETVAL_UNDEFINED?


Scott: Things did change in a big way when I landed my necko API changes. It 
used to be that the js would be evaluated at the time you constructed the 
channel, which was wrong. When I removed some arguments from the constructor 
this then became impossible. The evaluation now occurs at the point you "get the 
data" i.e. AsyncRead or OpenInputStream. That means the check for 
NS_ERROR_DOM_RETVAL_UNDEFINED needs to move to the point where the content data 
starts coming back. I assume that's OnStopRequest, or OnEndDocumentLoad or 
somewhere like that. And that someplace needs to be the decision point for 
whether we clear the current document and build a new content viewer or not.

I think maybe we should get together to step through this.
*** Bug 35026 has been marked as a duplicate of this bug. ***
Whiteboard: 1d
*** Bug 35151 has been marked as a duplicate of this bug. ***
*** Bug 34260 has been marked as a duplicate of this bug. ***
*** Bug 34805 has been marked as a duplicate of this bug. ***
I see this bug with winNT build 2000041008.
Also this affects yahoo.com especially their webmail.

Keywords: beta2, top100
OS: Linux → All
Blocks: 27367
bumping up the priority and severity, so i don't ship m15 without this fixed.
Severity: major → critical
Priority: P3 → P1
*** Bug 35571 has been marked as a duplicate of this bug. ***
mozilla segfaults on todays build (20000041309) (click on one of the pics) 

failed to set the page title.
Error loading URL http://www.maximonline.com/girlfriend_of_day/index.asp 
Document: Done (0.239 secs)
.//run-mozilla.sh: line 29:  6936 Segmentation fault      $prog ${1+"$@"}

*** Bug 35784 has been marked as a duplicate of this bug. ***
related to bug 35794 possibly?
*** Bug 33973 has been marked as a duplicate of this bug. ***
Blocks: 32640
Warren? any more status?
I came up with a hack that allows the js evaluation to occur earlier so that we 
can decide whether to call AsyncRead or not. This will prevent the 
current document from getting destroyed. I'll attach it, but mscott or travis 
should probably review it before I land it.
Thanks warren, i've applied the patch (by hand, apparently, the files have
changed a bit since we branched =) to a branch tree, and i'll report as soon as
i've given it a test run.
I pulled that branch last night around 3 am, and that's what the diffs are for. 
I wouldn't have suspected it has changed since then.
ok, i don't know what i did wrong in applying the diffs, but they were small
enough to apply by hand, so i did. The resultant build for me on linux is just
as stable as the previous m15 builds, and i can do cool things like add sidebar
panels and whatnot.

Warren, i'm not sure how much of a review you think is necessary, or how much
testing we need pre-checkin, but i'm comfortable with you checking this stuff
in. I'm tempted to do it myself right now, so we can see this stuff in the
release builds tomorrow, but i'll hold off, and let you commit.
Committed to SeaMonkey_M15_BRANCH. Thanks for all your help Leaf.

Note: we need to leave this bug open for the real fix to the tip. Moving to 
M16.
Target Milestone: M15 → M16
*** Bug 35938 has been marked as a duplicate of this bug. ***
Warren, I don't know much about the patch that was put in the m15 branch, but 
how dangerous would it be to have that patch put on the tip for nightly builds?
At least until the real fix can land?

That would stop dups and make testers happy :)


The tip (non-m15 branch) is completely different now, and the same patch won't 
work.
*** Bug 33787 has been marked as a duplicate of this bug. ***
Keywords: nsbeta2
Blocks: 36333
*** Bug 36404 has been marked as a duplicate of this bug. ***
This totally sucks and I'm tired of it!  Adding dogfood keyword.
Keywords: beta2dogfood
Making [nsbeta2+] with [dogfood-].
Whiteboard: 1d → [nsbeta2+][dogfood-]1d
Fix checked in.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
I verified this one with 04-21-09[M16] builds, as well as 04-18-06[M15] builds, 
and its working fine now. JS functions are getting executed in hrefs [Javascript 
in URI].
Marking verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: