javascript: js in <a> tag clears window when executed

VERIFIED FIXED in M17

Status

()

P1
major
VERIFIED FIXED
19 years ago
18 years ago

People

(Reporter: sean, Assigned: mscott)

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2+], URL)

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
Clean build pulled 6/23 around 10:30am pacific.

Possibly a regression of http://bugzilla.mozilla.org/show_bug.cgi?id=34217.
Slightly different as the js DOES execute, but the window is cleared and the 
location gets set to the script that ran.
(Reporter)

Comment 1

19 years ago
nominating nsbeta2
Keywords: nsbeta2

Comment 2

19 years ago
Using "javascript:..." as the URL parameter to nsIPluginManager::GetURL() with 
a target of "_self" also shows the same behaviour. Setting 
the 'Force_Javascript' parameter to true doesn't fix the problem
Cc:ing valiski since his checkin on 06/16/2000 14:46 to mozilla/netwerk/base/
src/nsFileTransport.cpp, v1.79, broke the javascript: URL's. Also cc:ing warren
since he fixed this problem once before.
This simply *must* be fixed for beta2, period.
Priority: P3 → P1

Comment 4

19 years ago
that change 
(http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&f
ile=nsFileTransport.cpp&root=/cvsroot&subdir=mozilla/netwerk/base/src&command=DI
FF_FRAMESET&rev1=1.78&rev2=1.79) was to fire OnStart() *always.* The consumer 
needs to accommodate the OnStart() that it wasn't getting before.

Comment 5

19 years ago
Putting on [nsbeta2+] radar for beta2 fix. 
Whiteboard: [nsbeta2+]

Comment 6

19 years ago
*** Bug 43708 has been marked as a duplicate of this bug. ***

Comment 7

19 years ago
*** Bug 43708 has been marked as a duplicate of this bug. ***

Comment 8

19 years ago
jud: who is the *consumer* here? the webshell? so who gets this bug? rpotts?
(Reporter)

Comment 9

19 years ago
*** Bug 44214 has been marked as a duplicate of this bug. ***
Blocks: 43606

Comment 10

19 years ago
not sure actually, mscott maybe?
*** Bug 43851 has been marked as a duplicate of this bug. ***
nsDocumentOpenInfo::OnStartRequest() (in uriloader/base/nsURILoader.cpp) needs
to be able to get at the NS_ERROR_DOM_RETVAL_UNDEFINED error code returned from
nsJSThunk::Open() (in dom/src/jsurl/nsJSProtocolHandler.cpp), called from
nsFileTransport::Process(). If that's possible then we could check for that and
do nothin in OnStartRequest() in that case, this would fix this problem unless I
missed something here...

Comment 13

19 years ago
->mscott
Assignee: gagan → mscott
(Assignee)

Comment 14

19 years ago
Here's the problem:
1) the file channel does indeed set it's status correctly using
NS_ERROR_DOM_RETVAL_UNDEFINED.

2) However, the JS channel is a nsIOStreamChannel which wraps the file channel.
The uri loader only sees the JS channel.

3) Unfortunately, the status for the JS channel is not correctly set to
NS_ERROR_DOM_RETVAL_UNDEFINED. It's always NS_SUCCESS. So the uri loader doesn't
know to not propogate the OnStartRequest.

This appears to be a bug between the file transport setting the right code and
reflecting that back up in the channel that wraps the file transport
(nsIOStreamChannel). Who owns that?
(Assignee)

Comment 15

19 years ago
I think it would work if nsStreamIOChannel::GetStatus was modified to instead of
looking at it's own mStatus, return the status of the file transport instead.
But this breaks the case where mStatus for the io channel has a value.

Or maybe, if mStatus is NS_SUCCESS(mStatus) then return the status of the file
transport. That would preserve the semantics of mStatus while allowing us to get
at the value for the transport channel.

Here's the current version:
NS_IMETHODIMP
nsStreamIOChannel::GetStatus(nsresult *status)
{
    *status = mStatus;
    return NS_OK;
}

I'm arguing that we need
if (NS_SUCCEEDED(rv))
  *status = the status for mFileTransport
else
  *status = mStatus;

Warren, do you buy off on that?
(Assignee)

Comment 16

19 years ago
Created attachment 11077 [details] [diff] [review]
proposed fix
(Assignee)

Comment 17

19 years ago
I re-implemented nsStreamIOChannel::GetStatus to return the status of the file
transport channel if it didn't have a non-error status code for its mStatus.

JS urls are now working again for me including this test case.

Warren, can you gimme a code review on the nsStreamIOChannel change?
Status: NEW → ASSIGNED
Target Milestone: --- → M17

Comment 18

19 years ago
You should check the result value of aChannel->GetStatus(&rv) in 
nsURILoader.cpp just to be safe. Also, I think GetStatus should probably look 
like this:

  if (mFileTransport)
    rv = mFileTransport->GetStatus(status);
  else
    *status = mStatus;

(Assignee)

Comment 19

19 years ago
Hey Warren, I already added code to the uriloader to check for the status on the 
channel and to explicilty check for NS_ERROR_DOM_RETVAL_UNDEFINED. My problem 
was that the nsIOStreamChannel wasn't giving me this value. 

this change fixes the problem.

Can I consider this reviewed then? (including your suggestion) thanks man.
(Assignee)

Comment 20

19 years ago
Oh wait, Warren, the reason I didn't write the code the way you just did is 
because now you are losing mStatus if it was given an error code. If you search 
through the code in nsStreamIOchannel you'll see that in several cases mStatus 
is set with error codes (particularly in the cancel case). My patch works around 
that by preserving the value of mStatus and using it if it isn't a success 
value. By making your suggested change, we break the semantics of mStatus in 
nsStreamIOchannel and will incorrectly miss error values.

Comment 21

19 years ago
I thought I saw that mStatus was only set by cancel. We could almost remove it 
entirely since canceling one of these without a transport is pretty meaningless 
(could return an error). But your patch is ok with me.

Comment 22

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

Comment 23

19 years ago
Fix checked in. This test js url is now working like it should for me. 
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 24

19 years ago
*** Bug 44812 has been marked as a duplicate of this bug. ***

Comment 25

19 years ago
*** Bug 45004 has been marked as a duplicate of this bug. ***

Comment 26

19 years ago
verif.
Win2000 2000071008
Linux 2000071008
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.