146522 - Publishing command should be more accessible

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[Charles Manske]

Recent usability testing has revealed that the Publish command needs to be more
visible, especially when editing a new page or a local file. Currently, the 
Publish button is only visible on the toolbar when editing a remote file.
Also, 5 out of 6 users tested had trouble finding the "Publish" menuitem in the
File menu, even though they had the File menu open!

Comment 1

[Charles Manske]

Attachment: patch v1

In general, this simplifies the Save and Publish commands. 
With this patch they have separate keybindings and toolbar buttons.
In ComposerCommands.js
at @@ -335,39 +335,32 @@:
In the nsSaveCommand, fix enabling of command when editing remote file and
don't do publishing when Save command is executed when editing a remote file. 
at @@ -1352,22 +1369,19 @@:
In the SaveDocument() method, don't do publishing when Save is used while 
editing a remote file -- force the SaveAs dialog instead.
at @@ -1667,71 +1698,21 @@:
Eliminate the switching of Publish and Save buttons on toolbar and switching of

"Ctrl/Cmd+S" keybinding between Publish and Save commands.

editor.xul:
at @@ -141,16 +141,17 @@:
Move the "Export to Text" menuitem to the "Tools" menu (this is according to
the
latest menu specs).
at @@ -171,20 +172,20 @@:
Move the "H.Line" button to the end of the toolbar and hide both "Anchor" and
"H.Line" toolbar buttons to make room for the "Publish" button. 5 out of 6 
users tested did not understand what the Anchor button did, so given our target

audience, these two items are not important enough to merit taking up valuable 

space on the toolbar. Note that if we decide to remove just one, I favor 
keeping the H.Line button.

editorOverlay.xul:
at @@ -34,17 +34,18 @@:
Fix keybindings so "Ctrl/Cmd+S" is always for Save and "Ctrl/Cmd+Shift+S" is
always for "Publish"
at @@ -133,17 +134,17 @@:
Added accesskeykey for "Export to Text" command so we wouldn't have to move 
it "&fileexporttotext.accesskey;" between DTD files.
at @@ -309,20 +310,20 @@:
Reorganize the Save and Publish menuitems to improve visibility.
Current grouping is:
------------------------
Save
Save As...
Save As Charset...
Publish
Publish As...
---------------------
Export to Text...
Revert to last saved
----------------------

This patch changes this to:
-------------------------
Save
Save As...
Save As Charset...
-------------------------
Publish
Publish As...
-------------------------
Revert to Last Saved
-------------------------

at @@ -381,22 +382,22 @@:
The order of items in the "Insert Menu" was changed to put more important items

at the top and reflect the order of items in the toolbar.
at @@ -668,17 +669,17 @@:
Remove "hidden" from Publish button -- it is always visible.

Comment 2

[Kathleen :Brade]

I'd rather see the File menu organized like this:
...
-------------------------
Save
Save As...
Save As Charset...
Revert to Last Saved
-------------------------
Publish
Publish As...
-------------------------
...

I don't like groupings of 1 if it'd be ok to add it to an existing group.  The
above should help Publish stand out a bit more (than if it has a one-item group
below it).

Comment 3

[Kathleen :Brade]

Comment on attachment 84834 [details] [diff] [review]
patch v1

is there some reason the accesskey is added to the command instead of the menu
item?

Comment 4

[Charles Manske]

Re: comment #2: 
Yes, I agree that a item by itself in a group isn't good, but this "Revert"
item actually refers to both "Saved" and "Published" files, so you can't put it
with either of those groups. In fact, I meant to suggest that maybe we should 
change the menu text to just "Revert" since current wording implies it applies 
only to save? 
We could also simply keep "Export to Text" with "Revert..." as it is now.

Re: comment #3: As I tried to explain in patch comments, if I moved the accesskey 
to the menuitem, which moved from editorOverlay.xul to editor.xul, I'd have to 
move the DTD string as well. I was trying to minimize code changes and it's 
better to keep the label and accesskey definitions next to each other in the DTD
file.

Comment 5

[Jaime Rodriguez, Jr.]

Lowering impact to [ADT2 RTM], per ADT triage.

Comment 6

[Charles Manske]

Attachment: patch v2

After further review of usability tests and discussion with Composer and UE
staff, this patch reflects final decision for the File menu:
---------------------
Save
Save As...
Save As Charset...
---------------------
Publish
Publish As...
---------------------
Revert
---------------------
Export to Text...
Browse Page
Send Page
... (rest as before)

Differences from previous patch: "Revert" menuitem text simplified since it
applies after both saving or publishing files. Access key changed to "v".
We decided that moving "Export to Text" to the tools menu would hide the item
too much, so it remains in the File menu but moved to the group with 
"Browse page" etc.

Comment 7

[Akkana Peck]

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

r=akkana
I had some problems testing it, like the publish button greying out then not
coming back even if publishing failed, but that happened without the patch too;
Charley thinks that's fixed by the patch in bug 134883.

Comment 8

[Daniel Veditz [:dveditz]]

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

sr=dveditz

Since there's a UI text change please inform the l10n team and get whatever
approvals required.

Comment 9

[Charles Manske]

checked into trunk

Comment 10

[scottputterman]

adding adt1.0.1+ for everything but the string change. Please get drivers
approval and check into the branch.

Comment 11

[sujay]

verified in 6/5 trunk

Comment 12

[Michele Carlson]

l10n approved IF you check this into the branch by tomorrow. 
thanks

Comment 13

[chris hofmann]

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

a=chofmann for 1.0.1  add the fixed1.0.1 keyword after checking in to the
branch

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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

a=shaver for the 1.1a branch.  (Pretend I added a checkmark.)

Comment 15

[Charles Manske]

checked into mozilla1.0.1 branch

Comment 16

[sujay]

verified in 6/7 branch build.

Comment 17

[sujay]

removing fixed1.0.1