Closed Bug 135834 Opened 22 years ago Closed 22 years ago

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

Categories

(SeaMonkey :: Composer, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: TucsonTester1, Assigned: cmanske)

References

Details

(Keywords: dataloss, Whiteboard: PUBLISH [adt2 rtm][fixed in branch],custrtm-)

Attachments

(1 file, 5 obsolete files)

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.
Wow! this is wierd.
I see the problem only when I select the first item in the SiteName list.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: nsbeta1
Whiteboard: PUBLISH
Target Milestone: --- → mozilla1.0
Sarah, are you also experiencing this problem ?
Keywords: nsbeta1nsbeta1+
Whiteboard: PUBLISH → PUBLISH [adt3]
Ryan--any chance you could look at this?
Attached file patch v1 (obsolete) —
This was an important discovery. We are corrupting publishing data by the line:

var tempData = publishSiteData[i];
Keywords: patch, review
Whiteboard: PUBLISH [adt3] → PUBLISH [adt2] FIX IN HAND, need r=,sr=
Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email adt@netscape.com.  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.
Keywords: nsbeta1-
Attached patch patch v2 (obsolete) — Splinter Review
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()
Attachment #80519 - Attachment is obsolete: true
seeking reconsideration due to data loss/corruption
Keywords: nsbeta1-dataloss
OS: Windows XP → All
Hardware: PC → All
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
Attachment #80519 - Attachment is patch: false
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.
Attachment #80519 - Attachment is obsolete: false
Attachment #80519 - Flags: review+
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.
Attachment #80521 - Flags: review+
Attached patch patch v3 (obsolete) — Splinter Review
This patch contains the more general 'clone' method that handles sub-objects
as suggested by akkana.
Attached patch patch v3b (obsolete) — Splinter Review
Oops! tweaked the "better clone method" version.
Attachment #80873 - Attachment is obsolete: true
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 on attachment 80521 [details] [diff] [review]
patch v2

sr=hewitt
Attachment #80521 - Flags: superreview+
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.
"patch v2" checked into trunk.
new revision: 1.13; previous revision: 1.12
Comment on attachment 80521 [details] [diff] [review]
patch v2

checked into trunk
Attachment #80521 - Attachment is obsolete: true
Comment on attachment 80521 [details] [diff] [review]
patch v2

checked into trunk
Attached patch patch v4 (obsolete) — Splinter Review
Diff for updated "Clone()" method relative to current trunk tree.
Attachment #80875 - Attachment is obsolete: true
Attached patch patch v5Splinter Review
Ugg! wrong file last time.
New "Clone()" method relative to current trunk tree.
Attachment #81965 - Attachment is obsolete: true
Comment on attachment 81966 [details] [diff] [review]
patch v5

r=akkana
Attachment #81966 - Flags: review+
Are any comments going to be added to publishprefs.js?
Whiteboard: PUBLISH [adt2] FIX IN HAND, need r=,sr= → PUBLISH [adt2] FIX IN HAND, need sr=
Charley, are you close to getting sr= on this? Should sujay be testing what was
landed earlier on the trunk?
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.
Verified on the 05-09 trunk build.

This has been fixed.
Whiteboard: PUBLISH [adt2] FIX IN HAND, need sr= → PUBLISH [adt2 rtm] FIX IN HAND, need sr=
Attachment #80519 - Attachment is obsolete: true
Comment on attachment 81966 [details] [diff] [review]
patch v5

sr=hewitt
Attachment #81966 - Flags: superreview+
patch v5 checked into trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: PUBLISH [adt2 rtm] FIX IN HAND, need sr= → PUBLISH [adt2 rtm][FIXED_IN_TRUNK]
Verified on the 05-17 trunk.
Status: RESOLVED → VERIFIED
Please nominate this for adt1.0.0 since it is fixed on the trunk
Charley--please e-mail drivers and request to land this on the branch
Keywords: adt1.0.0, mozilla1.0
Blocks: 143047
Keywords: adt1.0.0adt1.0.0+, approval
Whiteboard: PUBLISH [adt2 rtm][FIXED_IN_TRUNK] → PUBLISH [adt2 rtm][FIXED_IN_TRUNK] [Needs a=]
Adding custrtm-; no impact on customization.
Whiteboard: PUBLISH [adt2 rtm][FIXED_IN_TRUNK] [Needs a=] → PUBLISH [adt2 rtm][FIXED_IN_TRUNK] [Needs a=],custrtm-
Keywords: patch, reviewmozilla1.0.1
Target Milestone: mozilla1.0 → mozilla1.0.1
a=chofmann for 1.0.1
checked into mozilla1.0.1 branch
Whiteboard: PUBLISH [adt2 rtm][FIXED_IN_TRUNK] [Needs a=],custrtm- → PUBLISH [adt2 rtm][fixed in branch],custrtm-
verified in 6/4 branch
Keywords: verified1.0.1
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: