136413 - publishing site name UI needs work

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[sujay]

filing this bug per Kathy and also some usability feedback from Lisa.

Site name field is causing some confusion in Publish dialog.

One solution Kathy suggests is perhaps populating it with a recommended
site name. 

Users have no idea that this site name field is just a mechanism
to give your publish transfer a name that you can later refer to
in the site settings.

Comment 1

[Kathleen :Brade]

The proposed fix is to create the site name as this:
  scheme host

Here's an example:
  ftp marvin.mcom.com

If that sitename is already in use, a number will be appended.

-->brade

Comment 2

[Ryan Cassin]

-->Hurricane

Comment 3

[sujay]

adding Lisa's comments from another bug:

------- Additional Comments From [e-mail]  2002-04-10 11:53 -------
My initial confusion was with Site Names.  I saw some URLs listed under site
names and thought that this field was where I would specify the publishing URL.
 I didn't think of going to the Settings tab.    I kept trying to figure out how
to modify the site name since it's just a drop down list.  I wanted to be able
to type in it.

Comment 4

[Ryan Cassin]

Attachment: Fix, implemented as suggested

This should do it... feedback welcome.

Comment 5

[Ryan Cassin]

Attachment: Screenshot of publish window with patch

This is a screenshot of the publish window when the patch is applied (in case
you want to see the patch in action but don't want to go to the hassle of
merging it into your tree). Note the site name text box is in the form "host
(scheme)"

Comment 6

[Kathleen :Brade]

Ryan--does this patch work if you rename your site to something else?  It looks
like it always sets the sitename to the host (scheme) instead of setting it only
when a new site is defined (perhaps I need more context)?

Comment 7

[Ryan Cassin]

Brade, I tried the following:
1. Open the composer and bring up the Publishing Site Settings dialog
2. Create a new site with the following information:
     Site Name: Mozilla
     Publishing URL: http://www.mozilla.org/
     HTTP address: http://www.mozilla.org
3. Click Ok
4. Go to File >> Open Web Location...
5. Open www.mozilla.org in a composer window
6. Once the page loads, go to File >> Publish As
7. Note the site name is "Mozilla" and the publishing url is "mozilla.org" 
(not http://www.mozilla.org like it is when you don't have a site defined).

IIRC, a loop looking for a known site is run before we even get to the host 
(scheme) code. We should be okay :) Hope that clears it up!

Comment 8

[Kathleen :Brade]

OK, I actually looked at your patch along side the checked in version and saw
the piece I was missing (more context in your patch is generally very useful).

r=brade

Comment 9

[Kathleen :Brade]

Comment on attachment 78639 [details] [diff] [review]
Fix, implemented as suggested

r=brade (though I'd shorten the comment (remove example) so it's < 80 col)

Comment 10

[kinmoz]

Comment on attachment 78639 [details] [diff] [review]
Fix, implemented as suggested

sr=[e-mail]

Comment 11

[Kathleen :Brade]

I have checked this in on the trunk for Ryan.
Hopefully we can land it on the branch later.

Comment 12

[Charles Manske]

Comment on attachment 78639 [details] [diff] [review]
Fix, implemented as suggested

Not complete.
We also have to test for duplicates to existing sitenames and append something
like a "-x" number as well.

Comment 13

[Kathleen :Brade]

reopen bug to address duplicate names and/or appending of numbers

Comment 14

[Charles Manske]

I'm going to take this back since I've discovered a more serious, but related 
problem: When user changes the sitename, we create a new entry in the publishing
database instead of simply changing the sitename. This will bloat and pollute the
site database in prefs with lots of useless sites.
I'll use the work that Ryan did for the new sitename creation algorithm. Thanks!

Comment 15

[Charles Manske]

Attachment: patch v2

Adds "previousSiteName" variable to publishData so user can change sitename
but we detect previous site in publish data saved to prefs and update that 
instead of adding a new site.
These lines were deleted:
-  if (siteCount > 1)
-    siteNameList.sort();
because the siteNameList was already sorted when it was obtained by this
existing code:
  // Array of site names - sorted, but don't put default name first
  var siteNameList = GetSiteNameList(true, false);

Comment 16

[Akkana Peck]

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

>-    if (selectedItem && selectedItem.getAttribute("label") != siteName)
>+    var oldName = selectedItem.getAttribute("label");
>+    if (selectedItem &&  oldName != siteName)

Most of it looks fine, but I don't understand this part.  You used to be 
checking selectedItem before dereferencing it, but now you're dereferencing
first, checking second.  Why, and is that safe?

Comment 17

[Charles Manske]

Attachment: patch v3

fixed issue found by akkana

Comment 18

[Akkana Peck]

Comment on attachment 80892 [details] [diff] [review]
patch v3

r=akkana

Comment 19

[Joe Hewitt (gone)]

Comment on attachment 80892 [details] [diff] [review]
patch v3

sr=hewitt

Comment 20

[Charles Manske]

checked into trunk
new revision: 1.12; previous revision: 1.11

Comment 21

[michael wendell]

Verified on the 05-02 trunk build.

Comment 22

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) for approval to checkin to the 1.0 branch, pending
Drivers approval.  After, checking in, please add the fixed1.0 keyword.

Comment 23

[Christine Wozniak]

Adding custrtm-; no impact on customization.

Comment 24

[chris hofmann]

a=chofmann for 1.0.1

Comment 25

[Charles Manske]

checked into mozilla1.0.1 branch

Comment 26

[sujay]

verified in 6/4 branch build.