publishing site name UI needs work

VERIFIED FIXED in mozilla1.0.1

Status

VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: sujay, Assigned: cmanske)

Tracking

Trunk
mozilla1.0.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: publish [adt2 RTM][fixed in branch],custrtm-)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

17 years ago
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

17 years ago
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
Assignee: cmanske → brade
Keywords: mozilla1.0, nsbeta1
Whiteboard: publish
Target Milestone: --- → mozilla1.0

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 2

17 years ago
-->Hurricane
Assignee: brade → rcassin
Status: ASSIGNED → NEW
(Reporter)

Comment 3

17 years ago
adding Lisa's comments from another bug:

------- Additional Comments From lchiang@netscape.com  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

17 years ago
Created attachment 78639 [details] [diff] [review]
Fix, implemented as suggested

This should do it... feedback welcome.

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: publish → publish [FIX IN HAND] need r=, sr=, a=

Comment 5

17 years ago
Created attachment 78642 [details]
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)"

Updated

17 years ago
Keywords: nsbeta1 → nsbeta1+
Whiteboard: publish [FIX IN HAND] need r=, sr=, a= → publish [adt3] [FIX IN HAND] need r=, sr=, a=

Comment 6

17 years ago
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)?
Keywords: nsbeta1+ → nsbeta1
Whiteboard: publish [adt3] [FIX IN HAND] need r=, sr=, a= → publish [FIX IN HAND] need r=, sr=, a=

Comment 7

17 years ago
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

17 years ago
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

17 years ago
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)
Attachment #78639 - Flags: review+

Comment 10

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

sr=kin@netscape.com
Attachment #78639 - Flags: superreview+

Comment 11

17 years ago
I have checked this in on the trunk for Ryan.
Hopefully we can land it on the branch later.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: publish [FIX IN HAND] need r=, sr=, a= → publish [FIX IN HAND] [adt3] [RTM]
(Assignee)

Comment 12

17 years ago
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.
Attachment #78639 - Flags: needs-work+

Comment 13

17 years ago
reopen bug to address duplicate names and/or appending of numbers
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

17 years ago
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!
Assignee: rcassin → cmanske
Status: REOPENED → NEW
(Assignee)

Comment 15

17 years ago
Created attachment 80608 [details] [diff] [review]
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);
Attachment #78639 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: publish [FIX IN HAND] [adt3] [RTM] → publish [FIX IN HAND] [adt3] [RTM] need r=,sr=

Comment 16

17 years ago
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?
(Assignee)

Comment 17

17 years ago
Created attachment 80892 [details] [diff] [review]
patch v3

fixed issue found by akkana
(Assignee)

Updated

17 years ago
Attachment #80608 - Attachment is obsolete: true

Comment 18

17 years ago
Comment on attachment 80892 [details] [diff] [review]
patch v3

r=akkana
Attachment #80892 - Flags: review+
(Assignee)

Updated

17 years ago
Whiteboard: publish [FIX IN HAND] [adt3] [RTM] need r=,sr= → publish [FIX IN HAND] [adt3] [RTM] need sr=

Comment 19

17 years ago
Comment on attachment 80892 [details] [diff] [review]
patch v3

sr=hewitt
Attachment #80892 - Flags: superreview+
(Assignee)

Comment 20

17 years ago
checked into trunk
new revision: 1.12; previous revision: 1.11
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: publish [FIX IN HAND] [adt3] [RTM] need sr= → publish [adt2 RTM]

Comment 21

17 years ago
Verified on the 05-02 trunk build.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

17 years ago
Keywords: nsbeta1 → nsbeta1+

Comment 22

17 years ago
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.
Blocks: 143047
Keywords: approval
Whiteboard: publish [adt2 RTM] → publish [adt2 RTM] [Needs a=]

Comment 23

16 years ago
Adding custrtm-; no impact on customization.

Updated

16 years ago
Whiteboard: publish [adt2 RTM] [Needs a=] → publish [adt2 RTM] [Needs a=],custrtm-
(Assignee)

Updated

16 years ago
Keywords: mozilla1.0.1
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 24

16 years ago
a=chofmann for 1.0.1
(Assignee)

Updated

16 years ago
Keywords: mozilla1.0.1 → mozilla1.0.1+
Whiteboard: publish [adt2 RTM] [Needs a=],custrtm- → publish [adt2 RTM] approved,custrtm-
(Assignee)

Comment 25

16 years ago
checked into mozilla1.0.1 branch
Keywords: mozilla1.0.1+ → fixed1.0.1
Whiteboard: publish [adt2 RTM] approved,custrtm- → publish [adt2 RTM][fixed in branch],custrtm-
(Reporter)

Comment 26

16 years ago
verified in 6/4 branch build.
Keywords: verified1.0.0
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.