Closed
Bug 73141
Opened 24 years ago
Closed 23 years ago
Clicking link in mail window brings up browser on home page
Categories
(SeaMonkey :: General, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.3
People
(Reporter: Bienvenu, Assigned: morse)
References
Details
(Whiteboard: [PDT+]fix on trunk, need testing)
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
If you start up mail w/o the browser, and click on a bugzilla link in a mail
message, the browser window opens and goes to the home page, not the bugzilla
page. Subsequent clicks on the bugzilla link does the right thing. CC'ing Alec
because he made some recent changes to the browser's uricontent listener code,
according to mscott.
Reporter | ||
Comment 1•24 years ago
|
||
actually, it starts opening the bugzilla page; then replaces it with the home
page. Reassigning to Alec per mscott's wish.
Assignee: mscott → alecf
Comment 2•24 years ago
|
||
I think this was jag's delayedInit stuff, which he just backed out..
Comment 3•24 years ago
|
||
looks like you get the same thing clicking on a URL in a mail message, or the
"Go to Bugzilla" or "Look at Bonsai" links on the bottom of the window (sorry, i
don't know all the terminology), after starting mozilla with "mozilla -mail"
Also for me it brings up a blank window instead of my home page, because that is
how my browser is configured to start up. In case that is useful information.
Comment 4•24 years ago
|
||
it seems that this SPECIFICALLY happens for mozilla -mail, not for just running
mozilla, launching mail, and closing all browser windows. This may be a command
line handling thing, not sure. Jag, any thoughts?
Comment 5•24 years ago
|
||
Yeah... It's because the browser doesn't know better than that it (the browser,
not mozilla as a whole) wasn't run yet and will thus try to do the "first time
browser starts up" thing. The solution is simple, just move the arguments[0]
check before the cmdLineURLUsed check (I've got it in a bug somewhere, let me
dig it up), but it will still do the "first time per session" thing when you
open a browser window with Accel-N. A better solution would be to move the first
time per session "url to load" magic into AppRunner, and just pass the url to
load in as arguments[0] (thus automatically getting the once per session).
Comment 6•24 years ago
|
||
hey alec, i never launch from mozilla -mail
I always bring up the browser first then launch mail.
I just happen to close the browser window later on. So no browser windows are
open when I see this.
Updated•24 years ago
|
Comment 7•24 years ago
|
||
so, I am still not seeing this, I've tried with and without -mail, and I have a
homepage. I have noticed that the url of my homepage appears in the urlbar just
before the bug appears... does it start loading for some people?
Reporter | ||
Comment 8•24 years ago
|
||
yes, it starts loading the home page, and then the second url comes along and
interrupts the first, it seems.
Comment 9•24 years ago
|
||
Using 2001041104/win2k, I no longer see this bug by running "mozilla -mail" and
clicking on a linkified bugzilla URL in a mail message.
However, I do still see it by running "mozilla -mail" and clicking "Go to
Bugzilla" or "Look at Bonsai" on the pop-up Mozilla menu on the Taskbar.
Still have Navigator set to launch to a blank page, so by "see the bug" I mean
that the Navigator window opens but stays blank instead of going to Bugzilla or
Bonsai.
Comment 10•24 years ago
|
||
*** Bug 76108 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Priority: P2 → P1
Comment 11•24 years ago
|
||
might be a dupe of bug 69521, or at least the patch there might fix this problem
Status: NEW → ASSIGNED
Comment 12•24 years ago
|
||
unfortunately, I did not even have time to look at this for moz 0.9. Moving to
0.9.1 and maintaining priority.. sorry!
Target Milestone: mozilla0.9 → mozilla0.9.1
Updated•24 years ago
|
Keywords: nsCatFood → nsCatFood+
Comment 14•24 years ago
|
||
*** Bug 82379 has been marked as a duplicate of this bug. ***
Comment 15•24 years ago
|
||
*** Bug 75689 has been marked as a duplicate of this bug. ***
Comment 16•24 years ago
|
||
You also get the home page / blank page / last visited (as per preferences) the
first time you choose "View Source" on an email message.
Comment 17•24 years ago
|
||
Doesn't necessarily just interrupt; it can prevent the load of the clicked
link. If your home page loads quickly (e.g., Google), it can finish loading and
displaying before the loading the link you clicked. What I'm currently seeing on
Win98 is that Google loads and displays, then the URL for the link I clicked on
flashes for maybe 1/5 second in location bar, then Google remains as URL and
page. Perhaps JS on the page that sets the focus prevents the correct link from
loading at all? Also, this isn't just bugzilla, clarifying summary.
Summary: clicking on bugzilla link in mail window brings up browser on home page → Clicking link in mail window brings up browser on home page
Comment 18•24 years ago
|
||
this is really eating my lunch in mail. I run into this every day =(. If you
can't reproduce it that way, bring up an aim window in the commercial tree.
Click on an http url in an an aim window (make sure you don't have a browser
window up). Happens every time then too. Just like in mail.
Comment 19•24 years ago
|
||
*** Bug 83409 has been marked as a duplicate of this bug. ***
Comment 20•24 years ago
|
||
so I finally spent some time investigating this. the problem is basically that
the code checks if .cmdLineURLUsed has been set, even if there was no url given
on the command line. the fix should be pretty easy, but might end up
reorganizing some code.
Comment 21•24 years ago
|
||
I think we need to bring this back onto the mozilla0.9.1 list. Alec, can you
look at it. thanks!
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Comment 22•24 years ago
|
||
I'm using Linux, Mozilla build 2001053108.
When I have just upgraded to the most recent build, I stumble upon a problem
which seems a lot like bug 73141 (or 75689).
Here's what happens:
With only the mail open,
1. Send yourself a message with hyperlinks (http://www.oeone.com)
2. Open that email and click on the hyperlink
3. It opens a Browser window, but it doesn't load the link (it remains blank,
but the link we clicked on shows in the URl field)
In case it's important, the default settings for the browser is to open on
www.mozilla.org.
Is my problem related to this one? Thanks for any help!
Comment 23•24 years ago
|
||
*** Bug 83438 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Whiteboard: no eta yet, will assess m0.9.1 risk after patch ready.
Comment 24•24 years ago
|
||
ok, I have found this to be more chaotic than I first thought. It seems that
when a link gets clicked on from mail, two things happen:
- a new browser window is opened
- the uri loader loads a url in that window
the problem is that these two actions happen asynchronously (don't even ask me
how we make sure the window has finished opening, in order to kick off the load)
after poking at this for a few hours, I've decided that ANY fix is going to be
very risky - there are just too many states that a new window can open in, and
its not worth risking those other cases to fix this one with a hacky fix.
I'm continuing to work on this, will hopefully have a patch for m0.9.2 soon
Whiteboard: no eta yet, will assess m0.9.1 risk after patch ready.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Comment 25•24 years ago
|
||
do we know what broke that caused this regression? maybe if we could figure that
out, a less risky fix would present itself?
Comment 26•24 years ago
|
||
we basically re-wrote the code which decides which url to load to account for
all the different cases (opening from the command line, opening via dde, opening
via uri loader, opening from a menu, etc, plus taking into account the users
preferences w.r.t. blank/homepage/last page visited) and this is the one edge
case. my concern is that if we fix this case, we will break another one. This
will be fixed for rtm.
My take on this is (sorry mail guys) that most people keep a browser window open
- i.e. you're not likely to be using mail if you're not using the browser as
well... so in the case of the average user, they will likely already have a
browser window open, and thus will not be plagued by this bug.
Comment 27•24 years ago
|
||
how web-centric of you. :) I use e-mail and keep it open all day, while browser
windows come and go as I need them only once in a while.
Comment 28•24 years ago
|
||
well, you're not the "most people" that I referred to above.
look, I'm going to fix it, I'm just providing the reasons for pushing off to 0.9.2
Comment 29•24 years ago
|
||
I'm not sure Alec's supposition is true of mail users in general, and certainly
not of me :-) (although we are admittedly a small minority in any case). When I
launch the product, I almost always bring up mail first, since my new messages
are much more interesting to me than any web page. I don't bring up a browser
window until I have a need for one, and generally that reason is to view links
in my mail, so this bites me a lot. That said, I agree with the current target;
this should not be a showstopper for beta.
Comment 30•24 years ago
|
||
If we really want this bug gone for 0.9.1, the simplest way to do that is to
create a browser instance object in the mail/news startup code and set
cmdLineURLUsed to true. Add similar code to composer if needed. This (ugly)
hack would be 3 lines of code (create object, set boolean, explicitely destroy
object).
The right way to fix this though is to make a browser window open to about:blank
unless it's passed an URI to load through window.arguments[0]. This however
requires a whole bunch of changes and is definitely post-0.9.1 material.
Comment 31•24 years ago
|
||
Well, Peter Trudelle, you can add my name to your "small minority" which is more
interested in Mailer than Browser. For the time being, Mozilla Mail is the only
mail client which can compete with Outlook Express, with its support of many
different languages as well as Unicode and most of all, Mozilla Mail is VB
Virus-free!
Comment 32•24 years ago
|
||
i haven't switched yet, but i'm like trudelle i use mail for quite a while
before i start browsing.
Comment 33•24 years ago
|
||
Add my name to the lsit of minority: in a work environment, emails are used
all-day, whereas web browsing is totally secondary. I don't consider Alec's
argument valid, although some other reasons (for example complexity of the fix
or the risk associated with it) may very well warrant the need to postpone to
0.9.2 Just my 2 cents worth...
Comment 34•24 years ago
|
||
please stop polluting this bug with "me too!"
for the 900th time, I'm going to fix this freakin' bug.
Updated•24 years ago
|
Whiteboard: 4 days, eta 6/15.
Comment 35•23 years ago
|
||
*** Bug 85727 has been marked as a duplicate of this bug. ***
Comment 36•23 years ago
|
||
ok, an update: I have a fix for this bug, but I leave for Boston in about an
hour. I will attach a fix the evening of June 18th (monday)
The quick 'n dirty solution, for those listening, is at the same time that we
check appCore.cmdLineURLUsed, we also check cmdLineService.URLToLoad to see if
there is any url passed on the command line
Updated•23 years ago
|
Whiteboard: 4 days, eta 6/15. → fix in hand, eta 6/18.
Comment 37•23 years ago
|
||
a further update - my fix didn't work as I thought, but I finally figured out
what's going on. When you click a link from mail that opens a new window, we're
passing in the URL via the nsBrowserContentHandler, but it's being ignored in
favor of the homepage..
again, I know how to fix this but I ran out of time today due to the barrage of
end-of-milestone super-review requests.. delaying this further...eta 6/20
Whiteboard: fix in hand, eta 6/18. → fix in hand, eta 6/20
Comment 38•23 years ago
|
||
Comment 39•23 years ago
|
||
ok, there were so many problems here, it's not even funny:
In nsBrowserContentHandler::HandleContent, we were calling wwatch->OpenWindow
with the spec of the url, and no arguments. When there are no arguments, then
the dialog is opened as if it is called from the standard window.open(), which
creates a window that has no element "window.arguments", which meant the newly
created window never saw the spec. This basically threw spec into a black hole
never to be seen again.
The result was that a browser window would be open, but the window would have no
clue as to what url it was supposed to be loading.
So, it would do its best guess and try to load the browser's homepage under some
circumstances, and by some wierd fluke, if there was no URL on the command line,
all subsequent windows after the first one would magically manage to load the
url that the user clicked on in mail.. but this is TOTALLY random and lucky that
that ever worked.
So, we needed to be able to create a navigator window with chrome, that
specified the URI that was clicked on in window.arguments[0].. but we don't want
the browser to try loading that uri, because the docloader will actually load
the uri moments later... and in fact, the load of that URI has already begun, so
it would be wrong of the browser to try to load it anyway!
So, we use another argument, window.arguments[2] (window.arguments[1] has other
uses) to pass a special string "dontload" which means that we should put the url
in the title bar, but not actually load it...
The patch attached does all of these things... please ignore the CMDLINEURL*
macro changes, I won't check those in - they are a part of another bug.
ideally, I'd get an r= from jag, and and sr= from mscott, but I welcome other
reviewers as well :)
Comment 40•23 years ago
|
||
actually, I think one thing I just forgot is that I think we're supposed to be
getting the browser chrome url from prefs.. investigating that now, but I'd
appreciate at least one reviewer on this stuff anyway.
Comment 41•23 years ago
|
||
ok, technically I am supposed to load this from prefs ("browser.chromeURL" for
my own reference), but there are about a million instances that don't...:) but
I'll fix this one instance anyway..
Comment 42•23 years ago
|
||
I have a few questions about this patch. Let's discuss this tomorrow.
Comment 43•23 years ago
|
||
Alec/Jag - how is this one going? shd we be still shooting for m0.9.2? I think
we should, as it is critical to get some good testing coverage on this patch.
thanks, Vishy
Whiteboard: fix in hand, eta 6/20 → fix in hand, need new eta.
Comment 44•23 years ago
|
||
Jag and I discussed 2 possible modifications to the above patch.. I tried the
first one, but suddenly my build has started acting wierd... still hope to have
chosen one of the two by the end of today
Comment 45•23 years ago
|
||
pushing out. 0.9.2 is done. (querying for this string will get you the list of
the 0.9.2 bugs I moved to 0.9.3)
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Comment 46•23 years ago
|
||
Comment 47•23 years ago
|
||
ok, I finally got tired of trying to make my other patch work and went back to
trying to repair the continually broken logic.. and somehow, I found the correct
combination and ordering of if() statements this time (I don't know why it was
so hard the first few times I tried)
The other patch is on the way to the correct solution, but this patch at least
makes the bug in question go away.
Basically what I'm doing is making sure that there actually is a url on the
command line to load, before we go trying to make use of the "cmdLineURLUsed"
attribute... and I copied the !turboMode logic into the 2nd if() so logically
turboMode will do the same thing as before..
looking for r=, sr= and then I'll land this on the trunk. jag, mscott?
Comment 48•23 years ago
|
||
looks good to me alec. r/sr=mscott
Updated•23 years ago
|
Whiteboard: fix in hand, need new eta. → fix in hand, need r=
Comment 49•23 years ago
|
||
should !appCore.cmdLineURLUsed
be !(cmdLineURLUsed in appCore && appCore.cmdLineURLUsed) ?
Comment 50•23 years ago
|
||
timeless: no. Appcore is known to have that attribute.
alecf:
+ var cmdLineService =
Components.classes["@mozilla.org/appshell/commandLineService;1"]
+ .getService(Components.interfaces.nsICmdLineService);
I think the code is more readable when the dots match up...
+ var cmdLineUrl = cmdLineService.URLToLoad;
+ if (cmdLineUrl && !appCore.cmdLineURLUsed && !turboMode) {
+ uriToLoad = cmdLineUrl;
appCore.cmdLineURLUsed = true;
+ }
+
+ if (!uriToLoad && !turboMode) {
+ var cmdLineHandler =
Components.classes["@mozilla.org/commandlinehandler/general-startup;1?type=browser"]
+ .getService(Components.interfaces.nsICmdLineHandler);
+ uriToLoad = cmdLineHandler.defaultArgs;
}
I think the logic becomes more clear when you move the !turboMode out of the two
ifs into one if like this:
var uriToLoad;
if (!turboMode) {
var cmdLineService =
Components.classes["@mozilla.org/appshell/commandLineService;1"]
.getService(Components.interfaces.nsICmdLineService);
var cmdLineUrl = cmdLineService.URLToLoad;
if (cmdLineUrl && !appCore.cmdLineURLUsed) {
uriToLoad = cmdLineUrl;
appCore.cmdLineURLUsed = true;
}
if (!uriToLoad) {
var cmdLineHandler =
Components.classes["@mozilla.org/commandlinehandler/general-startup;1?type=browser"]
.getService(Components.interfaces.nsICmdLineHandler);
uriToLoad = cmdLineHandler.defaultArgs;
}
}
This would (theoretically) still not fix it if someone went and did something
like ./mozilla --mail http://myhomepage.com/
I wonder though if that would be considered a cmdLineUrl.
Assuming that you've tested browser startup; mail startup -> browser; mozilla -f
bloaturls.txt; r=jag
Updated•23 years ago
|
Whiteboard: fix in hand, need r= → fix in hand, landing on trunk
Comment 51•23 years ago
|
||
alec just checked this into the trunk. Adding the vtrunk keyword so we can get
this QAed tomorrow and hopefully we can get on the nsBranch+ list.
Keywords: vtrunk
Comment 52•23 years ago
|
||
"open link in new window" just broke on Linux.
Can't open via context-menu - can't open with middle-mouse click.
I do get a new window, but with about:blank, not desired url.
Comment 53•23 years ago
|
||
Bug 89345 filed for regression.
Comment 54•23 years ago
|
||
argh.. I need to move the window.arguments check above all the code that I just
checked in... I tested all the other options, but not anything involving
window.arguments. patch forthcoming in bug 89345
Comment 55•23 years ago
|
||
Comment 56•23 years ago
|
||
ok, I've attached a better patch which also fixes bug 89345, which was the
regression caused by the original patch.
Assuming the 2001-07-06 builds look good, then I'd recommend this patch goes
into the netscape branch.
Comment 57•23 years ago
|
||
How does this look on the trunk with the latest patch?
Comment 58•23 years ago
|
||
much better! open in new window and viewing the source of a mail message work
again, too.
Updated•23 years ago
|
Whiteboard: fix in hand, landing on trunk → fix on trunk, need testing
Comment 60•23 years ago
|
||
--> morse. Steve - could you take the last patch which Alec checked into the
trunk and keep it ready to check in on a branch tree. Once we get more testing
data, we'll see if we have an opportunity to merge this to the branch. thanks.
Comment 61•23 years ago
|
||
I've testeded this using trunk builds 2001-07-06 on linux and mac and 2001-07-09
on windows using the -mail to start. (for Mac I drop a bbedit page with ARGS:
-mail) on the Netscape icon). The scenarios mentioned in this bug and others
regarding blank pages for message source, links not opening up on first try all
seem to work now. I even clicked on a link in an IM from the Sidebar with only
Mail up. I then launched with Browser, selecting Mail app and closing all
Browser windows. This scenario works too. One thing I noticed on Linux, we
get an enabled Back button even though there is no Back, earlier today it went
into a unending loop, but now with the same build the Back goes to the Netscape
Home page. Not sure what was wrong when the loop was going on, maybe the Home
page was broken. In any case, that is different from Windows and Mac. I don't
think this should stop the bug. Still testing...will update if I find more.
Comment 63•23 years ago
|
||
OK, I finished testing the -mail scenarios. I downloaded trunk 2001-07-09 on
linux and mac and tested again. The probem with the new bug (89345) caused by
this one is gone too using -mail for launching.
The outstanding issue mentioned above with linux is still there (Back button
enabled). However, it only happens when you click on the link, not when you use
the context menu to open the link in a new window. Again, this shouldn't hold
up this bug unless someone see's an underlying problem with this behavior.
Verified on trunk, trunkverified
Comment 64•23 years ago
|
||
Is there a reason why we haven't gotten this PDT'plussed to check into the
branch yet? Looks like the testing happened back on 7/9. We are running out of
time. Can we make sure this comes up at today's meeting so it can get checked
in? morse/Vishy, can you guys bring this up at the meeting at 4pm today? Thanks.
Assignee | ||
Comment 65•23 years ago
|
||
I already made a request for pdt approval and am awaiting a reply
Comment 66•23 years ago
|
||
apparently PDT is very worried about possible side effects of this bug. I spent
some time looking at this code and I believe we've tested the potential code
paths that come down this route. We are as confident as we can be about the
fix.
Comment 67•23 years ago
|
||
as per PDT discussion today, we are not going to take this on the branch as it
is not a stopper per se and there is a workaround (click the link again).
marking fixed as this fix is on the trunk.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 68•23 years ago
|
||
*** Bug 90393 has been marked as a duplicate of this bug. ***
Comment 69•23 years ago
|
||
adding PDT+ per PDT. This can go on the branch now.
Whiteboard: fix on trunk, need testing → [PDT+]fix on trunk, need testing
Comment 70•23 years ago
|
||
Reopening, in that case.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 71•23 years ago
|
||
Fix checked in on branch.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 72•23 years ago
|
||
doron - I'm going to change the QA contact to Esther for verification on the
branch. (She's already verified on the trunk.)
If you want to take this bug back for your verifications, pls feel free to do
so. Thanks.
Comment 73•23 years ago
|
||
I still see this on branch builds win32 2001-07-15-08Branch..only the page which
pops up is blank, not the home page.
(The bug 90393 was duped for this one which covers the test case where the
browser windpws are closed and clciking a link in mail invokes a blank web page
with the correct link url in the search bar.)
Comment 74•23 years ago
|
||
Using build 2001-07-15 branch on windows and 2001-07-16 on mac and linux,
Stephen and I don't see this problem. Not sure why Suzanne is. We tested the
page source, open link in new window and clicking on a link in a mail message
after launching using -mail. I will check with Suzanne to see how she tested it.
Comment 75•23 years ago
|
||
The bug Suzanne logged is still happening. This bug was specific to launching
with -mail and then the first time use of a browser page for the source or a
link in a mail message brought up a blank page or a Netscape home page there
after it gave the correct content. Those (3) scenarios are working OK now with
this build. Bug 90393 is different and is still broken. Suzanne will reopen
that one, this one will be verified.
Status: RESOLVED → VERIFIED
Comment 76•23 years ago
|
||
*** Bug 86102 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•