172716 - Browse button launches blank page

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2002.10.04 commercial trunk builds on linux rh7.2 and mac 10.2.1.

1. open a file in composer.
2. click the Browse button in the toolbar.

results: the resulting browser window is blank.

no js console output, unfortunately.

Comment 1

[sairuh (rarely reading bugmail)]

kathy, pls do reassign as needed. thanks!

Comment 2

[sairuh (rarely reading bugmail)]

whups, forgot to turn on the js/chrome debug pref for the profile i was using.
after that, i did get the following js console output after clicking the Browse
button:

Error: window.arguments[0].split is not a function
Source File: chrome://navigator/content/navigator.js
Line: 500

relevant or not?

Comment 3

[James Napolitano]

The Browse button works fine for me in Win98-SE, Mozilla 1.2alpha.

Comment 4

[Kathleen :Brade]

This is a regression caused by jag's checkin for bug 118835 (or so it would appear).

Comment 5

[jag (Peter Annema)]

I have a patch, let me attach it here.

Fix is simple, composer was depending on navigator stringifying the object that
was passed in.

Comment 6

[jag (Peter Annema)]

Attachment: Fix composer to give browser a string object, make browser stringify incoming objects just in case

Comment 7

[sairuh (rarely reading bugmail)]

*** Bug 173183 has been marked as a duplicate of this bug. ***

Comment 8

[Charles Manske]

Actually, Composer generally uses "GetDocumentUrl()" (which gets the url string 
from the document) instead of "window.location" or "window.location.href" This 
command seems to be the only user of the "window.location..." pattern. Sorry!
Please test using "GetDocumentUrl()" instead. It seems to work for me.

Comment 9

[Charles Manske]

Hmmm. But now I see that we are comparing to "browser._content.location.href"
when trying to find an existing browser window with the url we are previewing.
I wonder if it's safer to use "window.location.href" in these circumstances?

Comment 10

[Kathleen :Brade]

Comment on attachment 102092 [details] [diff] [review]
Fix composer to give browser a string object, make browser stringify incoming objects just in case

r=brade
I think it's fine to use window._content.location.href for this place; plus it
will be more consistent if this needs to land on the 1.0 branch

Comment 11

[jag (Peter Annema)]

Attachment: Use GetDocumentUrl and getBrowser().currentURI.spec instead

Comment 12

[Charles Manske]

Comment on attachment 102186 [details] [diff] [review]
Use GetDocumentUrl and getBrowser().currentURI.spec instead

r=cmanske

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 102186 [details] [diff] [review]
Use GetDocumentUrl and getBrowser().currentURI.spec instead

>+      var uriArray = ("" + window.arguments[0]).split('\n'); // stringify and split

window.arguments[0].toString().split() would have looked prettier, but I'm fine
with this.

sr=dveditz

Comment 14

[Kathleen :Brade]

-->jag since this is his patch

Jag--did this fix also land on the 1.0x branch to fix the problem there?

Comment 15

[Samir Gehani]

Nav triage team: nsbeta1+/adt3

Comment 16

[jag (Peter Annema)]

Doh, yes, this made it onto the branch. It must've slipped my mind for the
trunk, I thought I had checked it in but bonsai says otherwise.

I'll try and get it in.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 102186 [details] [diff] [review]
Use GetDocumentUrl and getBrowser().currentURI.spec instead

a=asa for checkin to 1.2 (on behalf of drivers).

Comment 18

[Terri Preston]

This is fixed on win XP branch build 2002102209, linux branch build 2002102207
and Mac OS X branch build 2002102105, adding verified1.0.2 keyword

Comment 19

[Kathleen :Brade]

*** Bug 175519 has been marked as a duplicate of this bug. ***

Comment 20

[Kevin McCluskey (gone)]

Removing EDITORBASE, does not match the criteria for editorbase.

Comment 21

[sairuh (rarely reading bugmail)]

looks like this has already been checked in on the trunk (at least with a 10/29
build). marking resolved --but do reopen if there are other things remaining to
be done here!

Comment 22

[sairuh (rarely reading bugmail)]

been fixed on trunk for a while. :)