Closed
Bug 88208
Opened 24 years ago
Closed 23 years ago
UI for publishing (publish/site settings dialogs)
Categories
(Core :: DOM: Editor, defect, P2)
Core
DOM: Editor
Tracking
()
VERIFIED
FIXED
mozilla1.0
People
(Reporter: Brade, Assigned: cmanske)
References
Details
Attachments
(1 file, 19 obsolete files)
40.96 KB,
patch
|
Brade
:
review+
kinmoz
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
This bug is intended to cover the design and implementation of a publish dialog
for Composer.
I am assigning this bug to mpt for his design effort.
mpt--please reassign back to me for implementation (unless you also want to
implement the dialog).
Comment 1•24 years ago
|
||
Brade, what options does (or will, by the time the dialog is implemented) the
Composer back end support for publishing? In particular, does it have the
ability to:
1. Upload files (e.g. images) which are included/embedded in the current page?
2. Upload all files in a user-chosen folder?
3. Include or exclude subfolders when uploading a folder?
4. Detect which local files are newer than their remote equivalents, and only
upload those which have changed?
And a final question: Composer 4.x's Publish dialog includes a field for
entering the page title -- whereas the publish UI for other editors, such as
FrontPage and Dreamweaver, do not. Was there any particular reason for this?
(Perhaps people were unwilling to enter a title before a file went onto the Web
where a title would matter; but it also makes the Publish dialog more
complicated.)
Comment 2•24 years ago
|
||
I have made an attachment to BUG #28792 for this, as I did not know of this bugs
existence at the time :) Let me know if you want it reattached here. Perhaps
this bug is not needed, and everything can be tracked in 28792.
Reporter | ||
Comment 3•24 years ago
|
||
Reporter | ||
Comment 4•24 years ago
|
||
Comment 5•24 years ago
|
||
brade, do you need these mock-ups implemented?
Assignee | ||
Comment 6•24 years ago
|
||
Time to start on this, so I'll take the bug.
Brian what time do you have to help with this?
Assignee: mpt → cmanske
Whiteboard: EDITORBASE (5 weeks)
Target Milestone: --- → mozilla1.0
Comment 7•24 years ago
|
||
Charley, I'm ready. Give me a spec and I'll start right away.
Assignee | ||
Comment 8•24 years ago
|
||
Comment 9•24 years ago
|
||
The xul looks good, a couple things (probably done before my landing):
- Why do EditorPublish.xul and EditorPublishOverlay.xul need dialogOverlay.xul
and dialogOverlay.js now that they're <dialog/>s? (By the way, the overlay
brings in the js, so any editor dialog that does this is trying to load the js
twice...)
- <dialog/>s don't need <keyset id="dialogKeys"/> anymore
- The default orientation for <tabpanels/> is vertical, so it doesn't need to be
specified
- <text/> is deprecated (it should have been removed, is it still working?)
- The event handler should be "onselect", not "onSelect"
- The style for MoreFewer button should be in a css file
- The default orientation for <groupbox/> is vertical, so it doesn't need to be
specified
- Does EditorPublishOverlay.xul need the html namespace on its <overlay/> tag?
In other words (since there obviously aren't any html tags in the dialog), does
EditorPublish.dtd use <html:br/> like some of the other dialogs? Doesn't look
like it.
- What is an onselect on a box supposed to do...?
Assignee | ||
Comment 10•24 years ago
|
||
Thanks for the review! All suggested changes have been made. "dialogOverlay"
includes as well as other older XUL cruft has been removed or converted to new
sytax.
The "onselect" on the hbox was leftover from an earlier version. I've removed it.
The reason for the style on "MoreFewer" is that in all the other uses in Composer
dialogs, the text is "More Properties / Fewer Properties", which certainly isn't
correct here. As an interim hack, I've use "More" and "Less" as the button text,
and put the style in the XUL since the button should be smaller than the
"properties" version, which does set its width in CSS.
This is a 'first draft' checkin and details like that will be fixed as it matures.
I'll attach a new patch after working out other details from brade's review.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.7
Assignee | ||
Comment 11•24 years ago
|
||
Fixes old xul found by Blake. Don't force input of page title suggested by
Brade.
Attachment #57618 -
Attachment is obsolete: true
Assignee | ||
Updated•24 years ago
|
Assignee | ||
Comment 12•24 years ago
|
||
"First draft" is ready to checkin
Whiteboard: EDITORBASE (5 weeks) need r=,sr= → EDITORBASE (5 weeks), FIX IN HAND, need r=,sr=
Comment 13•24 years ago
|
||
Comment on attachment 57732 [details] [diff] [review]
Update to incorporate reviewers suggestions.
You still have a few <text/> uses in that patch. Could you please fix those?
Other than that, sr=hewitt
Attachment #57732 -
Flags: superreview+
Assignee | ||
Comment 14•24 years ago
|
||
All <text are removed (Forgot about the EditorPublishOverlay.xul)
Reporter | ||
Comment 15•24 years ago
|
||
Comment on attachment 57732 [details] [diff] [review]
Update to incorporate reviewers suggestions.
r=brade
Attachment #57732 -
Flags: review+
Assignee | ||
Comment 16•24 years ago
|
||
First draft is checked in. Whoopy!
Keeping bug open for discussion and improvements of this UI.
Comment 17•24 years ago
|
||
Charley, can you summarize what you checked in (what's in and what's missing)? I
see the new UI but it appears to be non-functional. What type of feedback
(specifically) are you looking for with this check-in? Thanks.
Assignee | ||
Comment 18•24 years ago
|
||
You can use the "Publish" or "Publish As" commands to bring up the Publish
dialog. There you can create new site profiles that should be retained in prefs,
so nexttime you launch you should see them. You can also play with site settings
in dialog launched by "Edit | Publishing Site Settings...".
Again, the interface should work for creating and modifying site settings.
It is not hooked up to Brade's core publishing code yet, so you can't actually
publish anything.
Comments on the UI are welcome.
Missing in the UI are the "other files" support (i.e., images, css etc) in the
"More" section of the Publish dialog, as well as other setting suggested in the
original spec.
Comment 19•24 years ago
|
||
The dialog resizes quite a lot when switching between the "publish" and
"settings" tabs, causing the mouse-cursor to not any longer being located over
the smaller of these dialogs. This in turn frequently causes me to "loose" the
dialog, since I "raise windows mouseover". The dialog vanishes behind some other
window more often than not. (Sawfish WM, linux) The same phenomena might occur
on Windows using the unsupported "X-mouse" from "TweakUI".
Why not leave the advanced publishing options present at all times? That way the
dialog would have the same size regardless of chosen tab. Otherwise, the dialog
should resize based on a fixed upper left window position.
Assignee | ||
Comment 20•24 years ago
|
||
Work for 0.9.7 milestone is done.
Re: comments in #19:
I don't see the problems you describe in Windows builds. Switching from "Publish"
to "Settings" simply expands the height of dialog dialog and doesn't move
the upper-left corner position. Is this a problem only in Linux?
As to the "mouseover" problem, we are now following strict window parenting
rules for dialogs. I use Tweak UI, and have used the "raise windows mouseover"
feature in the past, but I don't see that option under my current Windows 2000
installation, so I couldn't test it.
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Assignee | ||
Comment 21•24 years ago
|
||
New featues are not "EDITORBASE".
Priority: -- → P2
Whiteboard: EDITORBASE (5 weeks) → (5 weeks)
Reporter | ||
Comment 23•24 years ago
|
||
To be clear, this bug is intended to cover these items:
* publish panel
- basic info
- more/fewer views
- directory info
* site settings
other publishing-related UI stuff will be covered in a new bug.
Summary: UI for publishing → UI for publishing (publish/site settings dialogs)
Assignee | ||
Comment 24•24 years ago
|
||
*** Bug 110450 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 25•24 years ago
|
||
This implements improvements in the main Publish dialog, Settings Dialog,
getting
and setting data in prefs, and in Publish/Publish As, and Save commands so they
use the publishing database to find existing publishing sites.
Attachment #50525 -
Attachment is obsolete: true
Attachment #50526 -
Attachment is obsolete: true
Attachment #57732 -
Attachment is obsolete: true
Reporter | ||
Comment 26•24 years ago
|
||
Comment on attachment 68586 [details] [diff] [review]
Next round of UI work
this patch needs work; Charley and I discussed many issues on AIM including:
* lack of try/catch in many places
* moving save functions to end of ComposerCommands.js
* FinishHTMLSource() should be moved in cmd_publish
* addressing some issues now instead of adding comments
* adding overall comments for how things work
* .toLowerCase() isn't required in some places anymore (GetScheme does it for
us)
* fix JS warnings including this assignment: publishData.siteName ==
publishData.PublishUrl;
Is it possible to split this patch up into separate pieces?
Attachment #68586 -
Flags: needs-work+
Assignee | ||
Comment 27•23 years ago
|
||
This implements saving and getting user password from Password Manager and
should
finish latest work in publishprefs.js
Attachment #68586 -
Attachment is obsolete: true
Assignee | ||
Comment 28•23 years ago
|
||
More changes per reviewers comments
Attachment #68869 -
Attachment is obsolete: true
Assignee | ||
Comment 29•23 years ago
|
||
The only changes relative to last patch are in publishprefs.js:
SetPublishString() method added and nsISupportsWString is used as pref type
rather
than "char".
Attachment #68962 -
Attachment is obsolete: true
Assignee | ||
Comment 30•23 years ago
|
||
more changes after review discussions.
Assignee | ||
Updated•23 years ago
|
Attachment #69047 -
Attachment is obsolete: true
Reporter | ||
Comment 31•23 years ago
|
||
Comment on attachment 69257 [details] [diff] [review]
Updated patch
* be sure to leave all of these set to false:
-const gShowDebugOutputStateChange = false;
* I don't understand why we enable the Publish command if IsUrlAboutBlank;
shouldn't we leave it disabled so users will choose Publish As... ?
* move this line closer to where it is used or place it with the global debug
output flags (in that vicinity):
+ const nsIWebProgressListener =
Components.interfaces.nsIWebProgressListener;
* move this line to be closer to where it is used also (inside
gShowDebugOutputStateChange?)
+ var requestSpec;
* I think these lines should be swapped so that the assignment is inside the
try:
+ url = uri.spec;
+ try {
* Is there some reason to keep this as separate functions:
StripUsernamePassword
StripUsernamePasswordFromURI
* do we need a try/catch around this line:
+ return prefsService.getBranch("editor.publish.");
* if this line fails, shouldn't catch return false?
passwordManager.addUser(publishData.publishUrl...
* I don't understand why this line is in ValidateData(). Can it be removed or
is it used later in the function (not in diff)?
+ var title = TrimString(gDialog.PageTitleInput.value);
r=brade on these particular files:
editor.xul
EditorPublish.xul
EditorPublishOverlay.xul
EditorPublishSettings.xul
EditorPublish.dtd
Attachment #69257 -
Flags: needs-work+
Assignee | ||
Comment 32•23 years ago
|
||
Concerning "why we enable the Publish command if IsUrlAboutBlank;"
We do that for "Save" as well. It simply lets them attempt to save or publish
a new file even if now changes were made. They will end up in the Publish As
dialog anyway. For consistency with how "Save" works, I think this is corrrect.
Concerning moving "const nsIWebProgressListener =" and "var requestSpec;"
they should be at the top and will be used by final code once the debug output
block is moved. (We will need to use requestSpec even outside the debugging
code.) I changed the debug output anyway in upcomming patch, so it
makes sense in that context.
The rest of the suggestions are fixed in next patch.
Assignee | ||
Comment 33•23 years ago
|
||
The only things changed are in ComposerCommands.js and moving the "url =
uri.spec"
line in editorUtilities.js, as suggested.
Attachment #69257 -
Attachment is obsolete: true
Reporter | ||
Comment 34•23 years ago
|
||
Comment on attachment 69305 [details] [diff] [review]
Update
r=brade with these changes:
* be sure all flags are disabled like:
+const gShowDebugOutputStateChange = true;
* remove some extra spaces
* remove this line:
+ var title = TrimString(gDialog.PageTitleInput.value);
Attachment #69305 -
Flags: review+
Comment 35•23 years ago
|
||
Attachment #69305 -
Flags: superreview+
Assignee | ||
Comment 36•23 years ago
|
||
Comment on attachment 69305 [details] [diff] [review]
Update
This has been checked in.
Attachment #69305 -
Attachment is obsolete: true
Assignee | ||
Comment 37•23 years ago
|
||
Implemented in this patch:
1. Separate Save and Publish commands (Save is only "local", Pubish is remote)
Add support for setting initial site site when launching Publish dialog.
2. Added Publish toolbar button (see bug 125128 for icons and CSS patches)
3. Implement "Publish to" submenu under File menu and toolbar button.
4. Improve Publishing Settings dialog to support passwords, use <listbox> an
fix
various bugs.
5. More factoring and improvements in publish prefs methods for smarter "one
click publishing" support.
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 38•23 years ago
|
||
Addresses brade's review comments:
1. Added "SetElementHidden()" utility so we check if element exists when hiding
things.
2. Backed out new "DoAfterSave()" method -- to many conditional differences to
factor into common method (in ComposerCommands.js).
3. Small spelling error changes
Attachment #70022 -
Attachment is obsolete: true
Assignee | ||
Comment 39•23 years ago
|
||
Breaking down patch for clarity. These changes are the same as those in "Next
round--update"
(http://bugzilla.mozilla.org/attachment.cgi?id=70199&action=view)
for publishprefs.js
Attachment #70199 -
Attachment is obsolete: true
Assignee | ||
Comment 40•23 years ago
|
||
Breaking down patch for clarity. These changes are the same as those in "Next
round--update"
(http://bugzilla.mozilla.org/attachment.cgi?id=70199&action=view)
for dialog-related files
Assignee | ||
Comment 41•23 years ago
|
||
Breaking down patch for clarity.
Further changes for Publish commands and UI:
1. Publish button on toolbar does not have a submenu of publish sites.
We want this button to enable / disable as the Save button does to show current
modified state of document, and the submenu is inconsistent with that action.
2. Reduce number of items in File menu to:
Publish [the "one click publish" command, same as on toolbar]
Publish to >
a submenu with:
Create or Change Publish Info... [launch publish dialog ]
-----------------
SiteName 1
SiteName 2
etc. for all the sites we have prefs for.
(the wording for the "Create..." menuitem is under discussion!)
Note that is the old "Publish As" action.
In the Publish dialog that is launched, we allow setting Page Title, filename,
and site settings, and can create a new site, but it is not as powerful as the
full "Publish Settings" dialog accessed by "Edit | Publish Site Settings" menu.
Reporter | ||
Comment 42•23 years ago
|
||
Comment on attachment 70335 [details] [diff] [review]
Improvements to Publishing dialogs
r=brade
Attachment #70335 -
Flags: review+
Assignee | ||
Comment 43•23 years ago
|
||
Kin and I found a problem in the RemoveSite() method in EditorPublishSettings.js:
New code is:
// Remove one item from site data array
gPublishSiteData.splice(index, 1);
// Remove item from site list
gDialog.SiteList.selectedIndex = -1;
gDialog.SiteList.removeItemAt(index);
// Check if we removed last item and reselect a site
if (index >= gPublishSiteData.length)
index--;
InitSiteSettings(index);
Comment 44•23 years ago
|
||
Comment on attachment 70335 [details] [diff] [review]
Improvements to Publishing dialogs
sr=kin@netscape.com
Attachment #70335 -
Flags: superreview+
Reporter | ||
Comment 45•23 years ago
|
||
Comment on attachment 70342 [details] [diff] [review]
Improvements to publishing commands, menuitems, and add toolbar support
r=brade assuming we don't change some of the strings (we should wait for RobinF
to review all of them rather than changing them now)
Attachment #70342 -
Flags: review+
Assignee | ||
Comment 46•23 years ago
|
||
Per review request, I added this to ValidateData() method in EditorPublish.js
to change sitename in list when user changed it in Settings Tab:
// Update selected item if sitename changed
var selectedItem = gDialog.SiteList.selectedItem;
if (selectedItem && selectedItem.getAttribute("label") != siteName)
{
selectedItem.setAttribute("label", siteName);
gDialog.SiteList.setAttribute("label", siteName);
}
Comment 47•23 years ago
|
||
Comment on attachment 70342 [details] [diff] [review]
Improvements to publishing commands, menuitems, and add toolbar support
sr=kin@netscape.com
Should SetSaveAndPublishUI() be moved out of editor.js since it is shared with
mail?
Attachment #70342 -
Flags: superreview+
Assignee | ||
Comment 48•23 years ago
|
||
Fixes GetSiteList() to correctly put default sitename first if requested.
After much thinking about brade's comments about GetMatchingUrlLength()
method and its usage, I can't come up with a different method that really
helps!
Instead I changed the name and added lots of comments to help explain the
logic.
Attachment #70333 -
Attachment is obsolete: true
Reporter | ||
Comment 49•23 years ago
|
||
Comment on attachment 70389 [details] [diff] [review]
Update to publish prefs code
r=brade though I'm still disappointed about using length for matching sites;
also dependent on new bugs filed to cover these issues:
* handling of multiple identical prefs (different logins or other reasonable
variation)
* handle case where user is editing url with user@ in it
Attachment #70389 -
Flags: review+
Comment 50•23 years ago
|
||
Comment on attachment 70389 [details] [diff] [review]
Update to publish prefs code
sr=kin@netscape.com
Attachment #70389 -
Flags: superreview+
Assignee | ||
Comment 51•23 years ago
|
||
Work or this milestone done. Changing to next to continue.
Target Milestone: mozilla0.9.9 → mozilla1.0
Assignee | ||
Comment 52•23 years ago
|
||
*** Bug 126272 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•23 years ago
|
Whiteboard: (5 weeks) FIX IN HAND, need r=,sr= → (5 weeks) FIX IN HAND reviewed
Assignee | ||
Comment 53•23 years ago
|
||
Includes fixes to bug 126784 and bug 127084
Attachment #70335 -
Attachment is obsolete: true
Attachment #70342 -
Attachment is obsolete: true
Attachment #70389 -
Attachment is obsolete: true
Assignee | ||
Comment 54•23 years ago
|
||
Comment on attachment 71899 [details] [diff] [review]
New bug fixes and changes to UI from discussions
Issues addressed in this patch:
1. Remove "Pulish to" menu and simplify "Publish As" command code
2. Change some wording and UI for the "Other directory" part of Publish Dialog
3. Add code to deal with username and/or password embedded in a publishing URL
4. Fix problems updating settings in the Publish Settings dialog
5. Fix problems detailed in bug 126784 and bug 127084
Assignee | ||
Comment 55•23 years ago
|
||
A few changes after reviewer comments
Assignee | ||
Updated•23 years ago
|
Attachment #71899 -
Attachment is obsolete: true
Assignee | ||
Updated•23 years ago
|
Whiteboard: (5 weeks) FIX IN HAND reviewed → (5 weeks) FIX IN HAND, need r=,sr=
Assignee | ||
Comment 56•23 years ago
|
||
Change relative to last patch is in editorUtilities.js:
I eliminated previous StripUsernamePassword(), replacing it with
modified version of GetAndStripUsernamePassword() in which the usernameObj and
passwordObj out param objects are optional.
All calls to "GetAndStripUsernamePassword" were changed to
"StripUsernamePassword"
Assignee | ||
Updated•23 years ago
|
Attachment #71955 -
Attachment is obsolete: true
Assignee | ||
Comment 57•23 years ago
|
||
More small changes per reviewer comments
Attachment #72665 -
Attachment is obsolete: true
Reporter | ||
Comment 58•23 years ago
|
||
Comment on attachment 72679 [details] [diff] [review]
Update (last, I hope?)
r=brade
Attachment #72679 -
Flags: review+
Assignee | ||
Updated•23 years ago
|
Whiteboard: (5 weeks) FIX IN HAND, need r=,sr= → (5 weeks) FIX IN HAND, need sr=
Comment 59•23 years ago
|
||
Attachment #72679 -
Flags: superreview+
Whiteboard: (5 weeks) FIX IN HAND, need sr= → (5 weeks) FIX IN HAND, reviewed
Comment 60•23 years ago
|
||
Comment on attachment 72679 [details] [diff] [review]
Update (last, I hope?)
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72679 -
Flags: approval+
Assignee | ||
Comment 61•23 years ago
|
||
checked in.
All new problems found in publishing UI should be filed as separate bugs.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: (5 weeks) FIX IN HAND, reviewed
Comment 62•23 years ago
|
||
looks good. excellent work Charley.
verified in 3/8 trunk build on windows
if anyone finds any new issues, file them as separate bugs.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•