42058 - Open Web Location fails to load when New Nav window selected

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[sairuh (rarely reading bugmail)]

summary sez it all. found this using 2000.06.09.08-m17 trunk bits [moz and
comm]; occurs on all platforms.

1. select File > Open Web Location.
2. enter a url.
3. select New Navigator window from the droplist.
4. hit Enter or click Open button (if the latter, it'll be greyed out on win32
due to bug 42045, but still functional).

expected: dialog should go away, then a new browser window should appear and
load the entered url.

result: dialog goes away... but no new window appears (and the url isn't
loaded). nothing really useful from the console:

WEBSHELL+ = 6
WEBSHELL+ = 7
*** foopy
WEBSHELL- = 6
WEBSHELL- = 5

Comment 1

[sairuh (rarely reading bugmail)]

nominating for beta2. this be an annoying regression. thanks to Doron for
catching this!

Comment 2

[Grace Bush]

seeing in branch build 2000060814M16 (mozilla build)

Comment 3

[Blake Ross]

this is the line that appears to do nothing:
window.opener.delayedOpenWindow(getBrowserURL(), "chrome,all,dialog=no", 
dialog.input.value );

It _is_ reaching that point, since it dumps ***foopy right before that.  Seems 
like a problem with getBrowserURL() -- I had it dump the value that 
getBrowserURL() was returning to the console, and everytime it's WEBSHELL- = 3
That, to me at least, doesn't seem to be correct.

Comment 4

[Blake Ross]

I think I have a fix for this

Comment 5

[Blake Ross]

Attachment: patch - please test on all platforms

Comment 6

[leger]

Blake...wild looks good!  Not dogfood, but nsbeta2+ it is.  don says...see law 
for a code review and help checking in.

Comment 7

[Blake Ross]

adding patch kw, needs review

I don't know if this is correct, but it seems to work (on win32, anyways).   
For whatever reason, getBrowserURL(), defined as a function in 
utilityOverlay.js, wasn't returning the correct path to navigator.xul.  Perhaps 
it didn't know what getBrowserURL() was, since it didn't reference 
utilityOverlay.js, and it didn't seem to inherit it.  Still, I'd think that 
would have generated an error if that was the case.  This patch would seem fine 
to me, 'xcept that since openLocation.xul hasn't changed in over a month, I 
don't know why it suddenly isn't working (I would think perhaps a getBrowserURL
() problem, except Help | About Mozilla uses that it and it works just fine).

btw: feel free to remove my name from the contributors list on this file if you 
check this in...it won't hurt my feelings :) I know a one-line regression patch 
isn't much.

Comment 8

[Blake Ross]

fixing typo in qa contact email addr (sariuh->sairuh)!

Comment 9

[Blake Ross]

mm..someone said that fix isn't working.  it's working on my comp, perhaps i 
changed something else while in the process. i'll look into it

Comment 10

[Bill Law]

I applied the patch and it works for me (on Win98).  It looks like this should 
fix the problem; getBrowserURL doesn't seem to be defined unless that <script> 
tag pulls in utilityOverlay.js here.

Go ahead and check in the fix unless the report of the problem still existing 
elsewhere turns out not to be a false alarm.

Does anybody know exactly when this broke and what change broke it?  It works in 
my May31 build and fails in my Jun07 build so it seems to have happened between 
those dates.  If we can track down the change that broke this, then we can look 
to see if there are other JS utility routines waiting to be called from 
elsewhere that are likewise broken now.

Comment 11

[Blake Ross]

yep, did appear to be a false alarm -- asa said he pasted it wrong.  I couldn't 
find anything on cvs blame about this, didn't even appear to be any changes to 
the file in a month.  I don't know what happened here.

unfortunately, I can't check this in, unless someone here is willing to vouch 
for me so I can get cvs write access (please?  my patches work, are simple, and 
don't turn tinderbox red :).  other people have said this works, however, so 
someone can feel free to check this in.

Comment 12

[Bill Law]

I'll check in the fix.  I thought you already had proper authorization, based on 
what Jan wrote.  I'll be glad to vouch for you re: future work.  Give whoever 
needs it my name or tell me who to talk to.

As for the breakage, I suspect it happened due to changes in files a couple 
layers removed.  Some overlay, or some .js pulled in by some overlay, probably 
changed.  I tried to track it down but it wasn't obvious.

Comment 13

[Blake Ross]

Thanks for vouching for me - I'm getting write access today b/c of you!

In any event, I just tried this on the newest win32 M16 release candidate 
(2000061312).  It doesn't work.  Does this fix need to be put into that branch, 
or are we accepting this as an M16 problem and relnot'ing it?  We need to do 
one of the other (fix or relnote).

Comment 14

[sairuh (rarely reading bugmail)]

woo! vrfy 2000.06.14.08-m17 on all/all [commercial].

Comment 15

[Bill Law]

Congratulations, Blake.  I say a release note for M16, but that's only because I 
don't want to pull/build an M16 branch (yet; I may have to if/when other 
problems are found).  I'm not sure what the process is for escalating bugs that 
should be fixed in the M16 release.

Comment 16

[Blake Ross]

Thanks!

I talked to leaf, and we decided to relnote it.  Turns out sairuh already added 
it to the m16 relnote bug (37172) on 6/9, though.

Comment 17

[sairuh (rarely reading bugmail)]

yeah, afaik this went only into the trunk [m17]...and m16 is out de door!