Closed
Bug 25821
Opened 25 years ago
Closed 21 years ago
javascript: URL links with TARGET not processed correctly.
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
WORKSFORME
Future
People
(Reporter: yamfe, Assigned: mscott)
References
()
Details
(Keywords: dom0, helpwanted, Whiteboard: [nsbeta2-][rtm-] r=brendan, sr=mscott)
Attachments
(4 files)
1.59 KB,
patch
|
Details | Diff | Splinter Review | |
1.59 KB,
patch
|
Details | Diff | Splinter Review | |
1.45 KB,
patch
|
Details | Diff | Splinter Review | |
187 bytes,
text/html
|
Details |
I am using Mozilla to access an Oracle Application and it
gives out those bugs:
1. There are additional text that are displayed each time tables are
used. The texte are '1x1space'. The text could have been there
for testing purposes only... Just in case. I prefer mentioning that
2. The bottom frames of the application use different colors. The
'green' colour is displayed properly while there is a bug displaying
'blue'
Hope this helps
Comment 1•25 years ago
|
||
The problem appears on the third page in this site: click all check marks, click
continue, then continue again. The "1x1space" appear where spacer gifs named
"1x1space.gif" should be displayed.
I've had this problem before as well, on sites such as
http://www.browndailyherald.com/.
It seems as though spacer gifs (perhaps spacer images in general? unlikely...)
don't like being rendered. The primary characteristic of spacer gifs is that all
their pixels (no matter what size the image is) are transparent. Perhaps there
is code in Moz which doesn't grok this and falls back by displaying the image name.
Comment 3•25 years ago
|
||
After marking all of the checkboxes and hitting the continue button, I'm getting
a javascript error and nothing happens. I don't know if the html is bad or it is
a javascript error. Reassigning to Norris.
JavaScript Error: ReferenceError: go_forward is not defined
Assignee: karnaze → norris
Comment 4•25 years ago
|
||
Change component to DOM and reassign.
Assignee: norris → vidur
Component: HTMLTables → DOM Level 0
QA Contact: chrisd → desale
Okay, here's the story:
There are two simplest testcases: one is when the image is missing (in the case
of browndailyherald.com, or my own sample,
http://www.thirdnipple.com/mozilla/missing_image.html) or when the image is
broken (in the case of
http://www.national.com.au:8001/apphloan/bin/g_images/1x1space.gif, which is a
way to get that image without surfing around).
In other words, I don't believe it's a DOM bug at all. It's more of an
"indecision" bug -- they didn't want to display the "broken-image" image, and
didn't want to display nothing at all (which is what NS4 does), so they
displayed the first part of the filename instead.
All that needs to be done is to come to a decision about what to display if the
image is busted or missing. Fixing the bug after that should be trivial.
Whiteboard: 2 "simplest" testcases found
Comment 6•25 years ago
|
||
The javascript problem that Chris Karnaze mentioned seems to occur because the
bottom frame of the page has a link with javascript: URL with a TARGET
attribute. We're currently ignoring the TARGET attribute. I don't know if this
contributes to the problem reported originally.
I'm going to change the Summary to reflect the javascript: problem.
Status: NEW → ASSIGNED
Summary: Additional text printed in table / backgroung colour in Frames → javascript: URL links with TARGET not processed correctly.
Target Milestone: M16
Vidur, these are two distinct bugs: the javascript bug currently reflected in
the summary (reported by karnaze) and the broken/missing image bug
(reported by yamfe@yahoo.com) which I've found testcases for.
Somebody ought to separate these bugs, but I don't know whether there is a
standard way of doing this...
Comment 8•25 years ago
|
||
Nominating nsbeta2. We have to start drawing a line on DOM0 backward
compatibility; these bugs are supposed to be a high priority for nsbeta2 per the
beta2 criteria.
need to add a "missing image" bug to split off that problem. ekrock, can you do
this?
Whiteboard: 2 "simplest" testcases found → [nsbeta2-]2 "simplest" testcases found
Comment 10•25 years ago
|
||
Tested above testcases tonight. This bug also shows up on Mac OS build M16-
2000052408. Perhaps this should be marked as an "All" platform bug?
iMac DV, Mac OS 9.0.4, MRJ 2.2, carbon libs 1.0.4.
Comment 11•25 years ago
|
||
The 'broken/missing image problem' has been seperately reported.
Pls use bug 42364 to continue discussion on that problem .Thnx.
Comment 12•25 years ago
|
||
M16 has been out for a while now, these bugs target milestones need to be
updated.
Comment 13•25 years ago
|
||
If we aren't going to fix this for nsbeta2, then this should *** definitely ***
be an nsbeta3 stopper. b.c. with DOM0 event handling. No brainer that we have to
fix this before FCS.
Keywords: nsbeta3
Comment 14•25 years ago
|
||
Re-assigning to mscott for further analysis as per the decision in our bug
triage meeting.
Assignee: vidur → mscott
Status: ASSIGNED → NEW
Assignee | ||
Comment 15•25 years ago
|
||
*ahem* unfortunately at the url specified by this url, layout is no longer
laying out the continue button so I can't reproduce the steps karnaze was
following.
Comment 16•25 years ago
|
||
Escalating to P2 as this is high profile backward compatibility. javascript:
URLs with TARGET for links have been supported since Nav2 IIRC. As such, this is
eligible for check-in through 9/21. mscott, do you think you could fix by then?
If so, please accept as [nsbeta3+].
Priority: P3 → P2
Comment 17•25 years ago
|
||
comparing priority of this w/other mscott bugs...
Whiteboard: [nsbeta2-]2 "simplest" testcases found → [nsbeta2-][b3 need info]
Comment 18•25 years ago
|
||
mscott is overloaded. Can anyone lend a hand with this or suggest another person
who could? If we don't get this fixed, javascript: URL links with a designated
TARGET won't be processed correctly for RTM. Bad, as this is JS 1.0 DOM0
backward compatibility. Help!!! Marking helpwanted. cc:ing folks who might be
able to help or suggest someone who can.
Keywords: helpwanted,
rtm
Comment 19•25 years ago
|
||
Anyone, mscott especially: if you make the URL a non-javascript: one, does the
targeted window or frame load? If so, what's different about javascript: URL
handling? Any pointers to code (LXR links) would be much appreciated. I have
not dived into the nsJSProtocolHandler.cpp pool in a while.
/be
Assignee: mscott → brendan
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M16 → M18
Assignee | ||
Comment 20•25 years ago
|
||
Here's the problem Brendan: The docshell evaluates JS urls in a special fashion
before we even invole the uriloader. The act of opening a JS channel actually
forces evaluation of the JS. Warren tried to change this behavior in the
JSProtocolHandler but ran into a whole slew of problems (I don't remember them
now) and was forced to go back to the current behavior.
Before the uriloader is invoked, we create a channel for the URI. In the normal
case, we then pass this channel into the uriloader which opens the channel,
discovers the content type and retargets the output to the correct consumer.
However, the JS Protocol Handler evalues the JS when you create the channel. So
the result has already been determined before the uriloader is even involed. In
fact, we don't involve the loader at all if the result of the channel creation
was: NS_ERROR_DOM_RETVAL_UNDEFINED.
Here's the relevant code in docshell: nsDocShell::DoURILoad
// open a channel for the url
nsCOMPtr<nsIChannel> channel;
nsresult rv;
rv = NS_OpenURI(getter_AddRefs(channel), aURI, nsnull, loadGroup,
NS_STATIC_CAST(nsIInterfaceRequestor*, this));
if(NS_FAILED(rv))
{
if(NS_ERROR_DOM_RETVAL_UNDEFINED == rv) // if causing the channel changed the
return NS_OK; // dom and there is nothing else to do
else
return NS_ERROR_FAILURE;
}
Somehow I got taken off the bug list, cc'ing myself again.
Target Milestone: M18 → M16
Comment 21•25 years ago
|
||
Thanks mscott (sorry about not cc'ing you, I took the bug -- hope you don't mind
that ;-).
Without reopening the can of worms that warren closed by having the JS
evaluation take place at channel creation time, I bet we can fix this: don't we
know the TARGET attribute? In the case of a non-javascript: URL, how does the
URI loader discover that another window is being targeted?
/be
Assignee | ||
Comment 22•25 years ago
|
||
The uriloader has a private method called GetTarget:
http://lxr.mozilla.org/seamonkey/source/uriloader/base/nsURILoader.cpp#509
this method does the work of finding the right window context to open the url
inside of before it calls AsyncRead.
Presumably, this method could be made public and the JS ProtocoolHandler could
attempt to call through it to get the right window context (which can be
GetInterace'd for a docshell) before evaluating the JS.
Updated•25 years ago
|
OS: Windows NT → All
Hardware: PC → All
Target Milestone: M16 → M18
Comment 23•25 years ago
|
||
B3 has passed. Clearing "b3 need info"
Whiteboard: [nsbeta2-][b3 need info] → [nsbeta2-]
Updated•25 years ago
|
Target Milestone: M18 → ---
Comment 24•25 years ago
|
||
Seems like this should be rtm-. Is there an argument that huge numbers of
people will experience this bug? It looks like a great thing to get on the
trunk though. Please update the status whiteboard to indicate your desire to
get into RTM or not.
Whiteboard: [nsbeta2-] → [nsbeta2-][need info]
Comment 25•25 years ago
|
||
We are having this exact same problem with BuyerXpert 4.0, and if not fixed
would prevent us from supporting Netscape 6 with our Bx4.0 release on 11/30. We
use the target field to update information from our catalog order frame on the
right to the shopping cart frame on the left. The left frame does not get
rendered with the updated information.
Would like priority reset to P1 and severity as blocker
Comment 26•25 years ago
|
||
To recreate the problem here's the URL
http://maidstone.red.iplanet.com:9090/NASApp/buyer/Login
enter mercury.com
login as bugsbunny1
password is bugsbunny1
click on buy
click on following links in order general catalog, office furniture, book01
click on first select box
click on add to cart
At this point the left hand frame does not get rendered with the shopping cart
information that should reflect the item that has been added to the cart.
Comment 27•25 years ago
|
||
Tompkins, I followed your instructions and I do get the item in the left frame
when, am I missing something?
Comment 28•25 years ago
|
||
Brendan, a real fix for this problem will require a redesign of the way we do
link targetting in the docshell/uriloader, the way it's done today seems just
really really wrong. The problem is that when you click on a link today we tell
the docshell (the one that the link is displayed in) that the link was clicked,
then the docshell does it's URI loading (including mucking with session history
for the docshell where the link is, even if the link has a target!!!), initiates
the URL load and passes the URL load, including the target, off to the
uriloader. Once the uriloader gets a responce from the net/file it looks for the
target for the URL load and if there's no target it opens up a new window and
hands the data off to that window.
The problem with javascript: URL's is that they're actually executed when the
docshell initiates the URL load and thus the context they execute in is always
the one where the link came from.
We can fix part of this problem fairly easily, today the docshell always passes
'this' as the interface requestor to NS_OpenURI(). NS_OpenURI() is where the JS
in javascript: URL's is executed, passing in 'this' in the docshellcauses the JS
to always execute in the initiating window. In stead of passing in 'this' to
NS_OpenURI() we can do a FindItemWithName() in the dochshell and pass in the
other docshell as the interface requestor and the JS will be executed in the
correct window even if there's a target in the link *as long as the target
exists*. If the target window doesn't exist when the link is clicked
FindItemWithName() won't find the window so we'd haveto open the new window at
that time and that could have unknown bad side effects that I don't feel good
about at this stage in the game.
I'll attach a patch that makes targetting of javascript: URL's work as long as
the target window exists when the link is clicked. This is only a bandaid for
hiding the real problem...
Are there security issues we need to consider here, what if someone targets a
javascript: URL at a chrome window, will the normal security checks catch that
or would we need more code to deal with such a case?
Comments? Should we shoot for limbo with this?
Comment 29•25 years ago
|
||
Comment 30•25 years ago
|
||
Cc'ing mstoltz to comment on the security hazards of your patch.
/be
Comment 31•25 years ago
|
||
I just spoke to Rick Potts about this and he thought it would be an acceptable
change for the branch but we decided to special case this for javascript: URL's
since nobody seems to really know exactly what the interface requestor that we
pass into NS_OpenURI() is used for so just to be on the safe side we should only
do the FindItemWithName() if a javascript: URL is loaded. I'll whip that change
into my patch...
Comment 32•25 years ago
|
||
Comment 33•25 years ago
|
||
Something's wrong with those patches -- there's a reeeeeeeeeally long line of
text above the added code, that's not a comment. Also, the second patch looks
just like the first?
/be
Comment 34•25 years ago
|
||
Works for me as well.....using 10-31-00-14-MN6
Comment 35•25 years ago
|
||
Comment 36•25 years ago
|
||
Oops, my bad, have a look at the latest attachment.
Comment 37•25 years ago
|
||
mstoltz: check me here, I think so long as the subject principals used to
compile and execute (evaluate via an XPCOM proxied call) the script from the
javascript: URI's path part come from the channel, never from the ifreq, jst's
change is as secure as before, and fixes the problem that the script doesn't
evaluate using the target window as scope chain.
Indeed,
http://lxr.mozilla.org/mozilla/source/dom/src/jsurl/nsJSProtocolHandler.cpp#127
gets the channel owner and QIs to nsIPrincipal. So provided the channel for a
javascript: URI always has as its owner the principal of the document in which
the URI was embedded as a link (and of course that the QI succeeds), we are ok.
/be
Comment 38•25 years ago
|
||
Downloaded 10/31 N6 latest build. Works for me now, in latest build.
Comment 39•25 years ago
|
||
Brendan,
Yes, this sounds all right to me. This change doesn't affect the assignment
of principals to the channel which happens in docshell.
Comment 40•25 years ago
|
||
jst: r=brendan@mozilla.org on your last patch, but keep a bug open on the horrid
strncasecmp that makes the fix a special case for javascript: URIs. It should
be a general case, on the trunk, by mozilla0.9 I say.
/be
Comment 41•25 years ago
|
||
Adding keyword relnoteRTM, since there are lots of URL's with Target could be
affected.
Keywords: relnoteRTM
Assignee | ||
Comment 42•25 years ago
|
||
I like jst's work around for now until Rick and I fix this for mozilla later on.
I'd also like to petition this for the rtm branch since so many commercial sites
use window targeting with JS urls.
sr=mscott
Whiteboard: [nsbeta2-][need info] → [nsbeta2-][rtm need info]
Comment 43•25 years ago
|
||
This has a r= and sr=, and should go to rtm+ asap.
Whiteboard: [nsbeta2-][rtm need info] → [nsbeta2-][rtm need info] r=brendan, sr=mscott
Comment 44•25 years ago
|
||
Ok, I'll set [rtm+] (hope I'm not stepping on anyone's toes).
This is yet another DOM level 0 compatibility bug that didn't get enough
attention (my fault as much as anyone's; I took it from jst's list a while ago
to "help", then got caught in "higher priority" stuff), and suddenly seemed to
break important pages late in the game. Those pages may seem to work without
the fix, but this is yet another bug that could turn into a Big Compatibility
Deal fast, next week or next month.
The fix helps greatly for frame targeting, and the only case it doesn't address
is new-window. I urge the pdt to take this bug for rtm.
/be
Whiteboard: [nsbeta2-][rtm need info] r=brendan, sr=mscott → [nsbeta2-][rtm+] r=brendan, sr=mscott
Comment 45•25 years ago
|
||
Please land it on the trunk and test with BuyerExpert 4.0. Bring it back when
we know how it performs.
Whiteboard: [nsbeta2-][rtm+] r=brendan, sr=mscott → [nsbeta2-][rtm need info] r=brendan, sr=mscott
Comment 46•25 years ago
|
||
The fix is checked into the trunk now.
Comment 47•25 years ago
|
||
jst, you are the man and I am not. I'll do what I can to help get this rtm++'d
and in Netscape 6.
/be
Assignee: brendan → jst
Status: ASSIGNED → NEW
Comment 48•25 years ago
|
||
jar's radar, at least, ignores NEW bugs. ASSIGNING for jst.
/be
Status: NEW → ASSIGNED
Comment 49•25 years ago
|
||
We don't ignore "NEW bugs.
We do watch for "rtm+" bugs.
Please get info on buyer expert, add to bug, and renominate.
Thanks,
Jim
Comment 50•25 years ago
|
||
jar: ah, sorry -- thought I saw my name on your list (I got your mail), and I
had no [rtm+] bugs (not this [rtm need info] one, which I reassigned to jst).
Buyer Expert 4.0 people have been contacted, looking for positive feedback.
/be
Comment 51•25 years ago
|
||
Consider me the BuyerXpert 4.0 contact. Our original problem related to using
the javascript target field to rendor in another frame disappeared when we tried
it on the 10/31 build. From a BuyerXpert 4.0 perspective we consider our
original problem (which we equated with this bug) to be resolved.
Comment 52•25 years ago
|
||
So this should go back to rtm+ now?
Comment 53•25 years ago
|
||
Tested on 11/02 (todays) build as well. The reason I did this, is I'm
not sure of your checkin/build policy/system, so am not sure where the
fix is at the moment. Is it being built, is it in 11/02 build, or?
Anyway on 11/02 build, everything still aok for BuyerXpert 4.0 as it relates to
this bug.
Comment 54•25 years ago
|
||
This fix was placed in the trunk so it is in 11/02's build.
This has met all the criteria that the PDT asked for, and it should go back to rtm+.
Assignee | ||
Comment 55•25 years ago
|
||
yes this should be rtm+. The limbo list is getting made right now. Hopefully we
didn't miss it.
Whiteboard: [nsbeta2-][rtm need info] r=brendan, sr=mscott → [nsbeta2-][rtm+] r=brendan, sr=mscott
Comment 56•25 years ago
|
||
rtm-, already working for BuyerExpert 4.0 in 10/31 build.
Whiteboard: [nsbeta2-][rtm+] r=brendan, sr=mscott → [nsbeta2-][rtm-] r=brendan, sr=mscott
Comment 57•25 years ago
|
||
BuyerXpert 4.0 works with the branch bits as well (using the download link
below). So our problem seems to be resolved.
ftp://sweetlou/products/client/seamonkey/windows/32bit/x86/2000-11-02-09-MN6
Mark
Comment 58•25 years ago
|
||
Dupe of Bug 51237 ?
(Mozilla confuses multiple ports on same machine)
Comment 59•25 years ago
|
||
My workaround for this wasn't taken into the rtm branch so I'm reassigning this
to mscott who will fix this the right way on the trunk...
Assignee: jst → mscott
Status: ASSIGNED → NEW
Comment 60•25 years ago
|
||
Removing tompkins as cc
Comment 61•25 years ago
|
||
Nominating nsbeta1. DOM0 b.c. and we'd like the "fixed the right way" fix to be
included for nsbeta1.
Keywords: nsbeta1
Comment 62•24 years ago
|
||
what's up with this bug? looks like we have pantloads of ns6.0 rtm keywords...
is this bug fixed on the trunk? if so, might as well close it...
Comment 63•24 years ago
|
||
marking nsbeta1-
Comment 64•24 years ago
|
||
I think this patch introduced a cross-domain vulnerability: bug 77485,
"function definition isn't subject to domain checks (exploit involves targeted
javascript links)".
Comment 65•22 years ago
|
||
another reference to see the problem:
http://www.severianoribeiro.com.br/
![]() |
||
Comment 66•21 years ago
|
||
![]() |
||
Comment 67•21 years ago
|
||
So what is the bug about at this point? The testcase worksforme. What's left
to do?
Comment 68•21 years ago
|
||
testcase wfm with Camino nightly build 20040208, Mac OS 10.3.2.
Comment 69•21 years ago
|
||
Worksforme in 1.6 -- did something regress?
/be
![]() |
||
Comment 70•21 years ago
|
||
Huh? What do you mean? Nothing seems to have regressed that I can see... then
again, no one seems to be clear on what the right behavior is:
1) Where should the JS be evaluated?
2) With what priveleges?
3) Where should the output be displayed?
At the moment, #3 is the target frame; #1 I'm not sure about (but it's easy to
test), #2 is the nsIPrincipal of the calling document.
Comment 71•21 years ago
|
||
bz: sorry, I thought someone was still claiming "broken". If not, then
shouldn't this be WFM. Any new problem goes in a new bug.
Regarding your questions:
1) Where should the JS be evaluated?
2) With what priveleges?
3) Where should the output be displayed?
The answers are well-known and documented by various books, and more primarily,
by the MozillaClassic code.
The only wrinkle in the latest testcase not supported back in the old Netscape
Mozilla codebase is the data: URL, but a javascript: URL evaluating to a string
beginning with < would have the same effect (in that case, there would be an
inner javascript: URL in the a href attribute value). The JS is evaluated when
the link is clicked, in the lower frame. The privileges of generated documents
come from the generator. The output goes in the link target.
Why is any of this unclear? Really, I'm not being snarky -- can you point out
some conflicting spec or implementation?
/be
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WORKSFORME
![]() |
||
Comment 72•21 years ago
|
||
I wasn't saying that it was unclear what _should_ happen. Just that it's
unclear to me what we do for #1. And that's unclear because nsDocShell.cpp is a
bloody undocumented mess.
Comment 73•21 years ago
|
||
Yeah, docshell hurts my eyes. I just reviewed a jst patch that had to touch it
(bug 68215). It's in need of an enema before mozilla2.0, for sure.
/be
You need to log in
before you can comment on or make changes to this bug.
Description
•