135834 - Publish page dialog autofills http address in site sub-directory.

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[TucsonTester1]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.9+)
Gecko/20020405 Netscape6/6.2.1+
BuildID:    2002040503

If an existing page is opened in Composer, edited and published, the publish
page dialog server information is initially "auto-filled" with the site http
address.  When an existing site name is chosen, the appropriate content replaces
the http address in the server information section, but the http address is then
auto filled into the "site sub-directory for this page" box and must be removed
for page to publish properly.

Reproducible: Always
Steps to Reproduce:
1.Install/Launch NS 6.2.1+
2.Navigate to http://www.yahoo.com (or any existing site without framesets).
3.Open page in Composer (ctrl-e).
4.Edit content of page.
5.Click file | publish as...
6.Click on Publish tab.
7.Select existing site name from drop-down menu.


Actual Results:  When the appropriate site name is chosen, the previously
auto-filled http address in the server information area is replaced with the
correct information.  However, the "site sub-directory for this page:" is then
auto-filled with the http address.  

Expected Results:  site sub-directory window should remain empty unless a
directory is typed in or chosen from the drop-down menu.

Comment 1

[Charles Manske]

Wow! this is wierd.
I see the problem only when I select the first item in the SiteName list.

Comment 2

[sujay]

Sarah, are you also experiencing this problem ?

Comment 3

[Kathleen :Brade]

Ryan--any chance you could look at this?

Comment 4

[Charles Manske]

Attachment: patch v1

This was an important discovery. We are corrupting publishing data by the line:

var tempData = publishSiteData[i];

Comment 5

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 6

[Charles Manske]

Attachment: patch v2

While the previous patch was adequate, the changes in this patch are correct
and should be included for sturdier code:
1. pubUrlFound and browseUrlFound should be false if the respective urls are
empty
2. matchedLength should always be > 0 if there was a real matched url.
Note that the block at @@ -695,30 +731,35 @@
is in the method FillInMatchingPublishData()

Comment 7

[Kathleen :Brade]

seeking reconsideration due to data loss/corruption

Comment 8

[Charles Manske]

Comment on attachment 80519 [details]
patch v1

Removing obsolete: 
This patch would be the most conservative minimum fix in case we are worried
about risk for beta

Comment 9

[Akkana Peck]

Comment on attachment 80519 [details]
patch v1

r=akkana on the simple/safe patch, provided that you add a newline at the end
of the file.

Have you asked to see whether there's already a Clone() function somewhere else
in the tree that might be in scope?  Though it's small enough that it doesn't
matter very much.

Comment 10

[Akkana Peck]

Comment on attachment 80521 [details] [diff] [review]
patch v2

r=akkana on the longer patch too.

BTW, that Clone method isn't general -- if the object contains other objects,
they won't be cloned.  A similar but more general method is here:
http://developer.irt.org/script/879.htm
It should be fine in this case, but it might be worth adding a comment to Clone
in case anyone might be tempted to use it for other purposes.

Comment 11

[Charles Manske]

Attachment: patch v3

This patch contains the more general 'clone' method that handles sub-objects
as suggested by akkana.

Comment 12

[Charles Manske]

Attachment: patch v3b

Oops! tweaked the "better clone method" version.

Comment 13

[Charles Manske]

Note to reviewers: Please review all 3 Patches: "v1", "v2", "v3b" We might
checkin one vs. another for trunk vs. branch. "v3b" is an alternative to the 
"Clone()" method that can be used instead that method in either "v1" or "v2".

Comment 14

[Joe Hewitt (gone)]

Comment on attachment 80521 [details] [diff] [review]
patch v2

sr=hewitt

Comment 15

[Kathleen :Brade]

I prefer patch v1 with patch v3b's version of editorUtilities.js.
I'd probably be happier with v3b if there were more comments in the code.

Comment 16

[Charles Manske]

"patch v2" checked into trunk.
new revision: 1.13; previous revision: 1.12

Comment 17

[Charles Manske]

Comment on attachment 80521 [details] [diff] [review]
patch v2

checked into trunk

Comment 18

[Charles Manske]

Comment on attachment 80521 [details] [diff] [review]
patch v2

checked into trunk

Comment 19

[Charles Manske]

Attachment: patch v4

Diff for updated "Clone()" method relative to current trunk tree.

Comment 20

[Charles Manske]

Attachment: patch v5

Ugg! wrong file last time.
New "Clone()" method relative to current trunk tree.

Comment 21

[Akkana Peck]

Comment on attachment 81966 [details] [diff] [review]
patch v5

r=akkana

Comment 22

[Kathleen :Brade]

Are any comments going to be added to publishprefs.js?

Comment 23

[Syd Logan]

Charley, are you close to getting sr= on this? Should sujay be testing what was
landed earlier on the trunk?

Comment 24

[Charles Manske]

Yes, Sujay can verify the original bug description -- it is now fixed.
I'm holding it open to get the better version of the "Clone()" method approved,
but it doesn't affect the original bug.

Comment 25

[michael wendell]

Verified on the 05-09 trunk build.

This has been fixed.

Comment 26

[Joe Hewitt (gone)]

Comment on attachment 81966 [details] [diff] [review]
patch v5

sr=hewitt

Comment 27

[Charles Manske]

patch v5 checked into trunk

Comment 28

[michael wendell]

Verified on the 05-17 trunk.

Comment 29

[Syd Logan]

Please nominate this for adt1.0.0 since it is fixed on the trunk

Comment 30

[Kathleen :Brade]

Charley--please e-mail drivers and request to land this on the branch

Comment 31

[Christine Wozniak]

Adding custrtm-; no impact on customization.

Comment 32

[chris hofmann]

a=chofmann for 1.0.1

Comment 33

[Charles Manske]

checked into mozilla1.0.1 branch

Comment 34

[sujay]

verified in 6/4 branch