Closed Bug 39089 Opened 25 years ago Closed 25 years ago

2nd browser window is tiny

Categories

(Core :: DOM: Navigation, defect, P1)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
Windows NT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: waterson, Assigned: adamlock)

References

Details

(Keywords: regression)

Attachments

(2 files)

proposed fix
1.04 KB, patch
Details | Diff | Splinter Review
better fix that special cases "javascript:" URIs
925 bytes, patch
Details | Diff | Splinter Review
To reproduce: 1. Start the browser. 2. Open Preferences, Debug, make sure "Disable XUL Cache" is not set. If it is, clear it, and restart. 3. Select, "File, New Browser". Expected: new browser window. Actual: tiny little window, containing only bookmarks.
this usually happens when somebody monkeys with load groups or the chrome protocol handler. any clues?
On Linux, *two* browser windows open as a result of choosing "File, New Window". So this could be a result of a race. Anyone tinker with events?
window's OpenInternal method is being called twice, without ever leaving the JS interpreter loop. This leads me to believe that somebody has botched the JS in navigator.js or one of the overlays, and that this is at least one of the problems. Could someone familiar with those files please have a look? Unfortunately, it *still* looks like somebody has screwed brutal sharing, because a) the window's size varies from platform to platform, indicating that some kind of strange race is occuring, and b) the second time you bring up the "Preferences" dialog, it is totally hosed. Specifically, it only shows the "General" pref pane: no tree view with which to navigate the other panels. Again, could someone familiar with those problems please have a look? thanks, chris
jbetak found the problem with the two windows opening; however, on windows, the second window (of almost any kind) is always screwed up.
Is this really blocking anyone?
reassigning to danm as p1 for m16
Assignee: trudelle → danm
Priority: P3 → P1
Target Milestone: --- → M16
no, i suppose it's not blocking anyone. just means you can only ever open one window of any type. kinda makes it hard to send more than one email, for example. On a positive note, it's a great way to get rid of those annoying popups.
Severity: blocker → critical
Keywords: dogfood
This worksforme on Win98, using the respin verification bits - 051210
Are your prefs set s.t. your XUL Cache is enabled?
It seems like the prototype document is losing its overlay references...
Ok, it turns out that this is the same old "referrer" problem that comes back and bites brutal sharing now and then: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#2531 I have no idea why this is "needed for now", or what it does, but it's causing brutal sharing to bust. The "original URI" IS NOT THE REFERRER! It is the URI before any translation or redirection happened! How do we fix this?
Keywords: nsbeta2
cc jud.
cc'ing adam
Now it fails for me, exactly as described, same bits, same OS. Maybe I shouldn't have inhaled...
changing component to docshell, reassign to adamlock, add regression keyword
Assignee: danm → adamlock
Component: XP Toolkit/Widgets → Embedding: Docshell
Keywords: regression
*** Bug 39076 has been marked as a duplicate of this bug. ***
Attached patch proposed fix — — Splinter Review
mscott, adamlock, gagan: please take a look at the attached patch and let me know if it seems reasonable. Specifically, is the explicit set of the HTTP referrer field necessary?
Okay, so here's the scoop. Yes, http needs the referrer explicitly set on it becauase the reffer is different from the original URI. But there is code already in nsDocShell::DoURILoad to do this. So I'm not sure why you added this code again as part of the patch. given that, your change is just to modify SetOriginalURI to pass in the URI instead of the referrer. This is technically the correct behavior that we should be doing anyway. I tried to do this once before but I remember running into problems in the js protocol handler where it was expecting the refferrer to be in the original uri slot for a channel. So I couldn't check this change in. I was waiting on Norris to make changes to the JS protocol handler. But now Norris is gone...Hmmmm.... Have you tried an JS urls since you made this change in your tree?
This effect also occurs if you open prefs twice, or try to open AIM standalone twice, 2 chat windows, etc. etc.
It seems fine to me as long as aReferrerURI is not null. I notice the original tested for null but the new code doesn't.
Per my comments above, I believe this change will break JS urls because they need the referrer in the original URI slot. In nsJSProtocolHandler.cpp around line 126: // Get principal nsCOMPtr<nsIPrincipal> principal; nsCOMPtr<nsIURI> referringUri; // XXX this is wrong: see bugs 31818 and 29831. Norris is looking at it. rv = mChannel->GetOriginalURI(getter_AddRefs(referringUri)); we'll get back the wrong url after this patch and I believe we'll then run into problems because we'll be using the wrong referring url. Norris was going to fix this abose of the get original uri method. I'll see where he left these bugs at b4 he left.
See Bug #31818 which describes how we'll break JS urls if the docshell stops setting the refferrer as the original uri. In fact I have a separate bug to fix docshell for this (basically the same change waterson is proposing in this bug here) but I'm not able to check in that fix because of 31818.
mscott: Ok good, so explicitly setting the referrer *again* is not necessary. I'll remove that code. As for JavaScript URLs, well...no, they appear not to work with this fix. But they don't work *without* the fix either. And even if they did work, TOO BAD. This bug is worse.
One of the side effects of breaking JS urls with this change is that you will turn the smoketests orange as they rely on running JS urls to load urls during the smoketest (I learned that the hard way).
mscott, you are right re: smoketests. Okay, I'll mark this a dup of the other bug, and go tell mstoltz he's gotta fix JS URLs pronto.
Depends on: 31818
I talked with mstoltz on Friday, and he said it was unlikely he could patch JS URLs before M16. So...here's a hack. It'll do the old bad thing if you're trying to load a JS URL, otherwise, it'll do the happy shiny right thing. I've tested this vs. the bloaturls and it works (i.e., doesn't cause the bloaturls to hang). And best of all, it unbreaks brutal sharing. mscott, adamlock: could you review for me?
This temporary solution works for me.
thanks mscott. checked in the fix. mstoltz: please remember to *undo* this hackery when you land the real fixes for 31818!
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
*** Bug 39185 has been marked as a duplicate of this bug. ***
*** Bug 39092 has been marked as a duplicate of this bug. ***
*** Bug 39049 has been marked as a duplicate of this bug. ***
Trying to verify, has the "real" fix been landed?
Verified 092711 Win32.
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: