Remove Undo command in Manage Bookmarks Edit menu

VERIFIED WONTFIX

Status

SeaMonkey
Bookmarks & History
P2
normal
VERIFIED WONTFIX
18 years ago
13 years ago

People

(Reporter: Blake Ross, Assigned: matt)

Tracking

({relnote})

Trunk
relnote

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: relnote-user [rtm-]Fix in hand, reviewed and approved)

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
Build ID: just pulled

The Undo command in the Edit menu of the Manage Bookmarks window doesn't work,
and will not work for Netscape 6.0.  It should be removed from the window before
rtm, so as to not confuse users into thinking that they can get bookmark items
back if they delete them.
*** Bug 53804 has been marked as a duplicate of this bug. ***
*** Bug 53801 has been marked as a duplicate of this bug. ***

Comment 3

18 years ago
DON'T remove this menu item...  (it just needs to be disabled, as long as 
bookmarks don't support Undo).
(Reporter)

Comment 4

18 years ago
What?  So we should leave menuitems in the product that we know are going to 
remain permanently disabled, just in the hopes that they will someday work 
later on?  Okay then, who's going to generate a list of all the features we 
plan on having in the feature?  after all, we need to insert disabled menuitems 
for all of them.

If the menu item isn't going to work, let's not pretend it might at some point.
(Reporter)

Comment 5

18 years ago
nominating for rtm. unless someone can explain why we should trick the user 
into thinking they can do certain things, let's easily remove this item for 6.0.
Keywords: rtm

Comment 6

18 years ago
Doh!  This is embarrasing.  How many other windows have non-function undo
commands?  Anybody wanna make a guess?  Marking the "rtm+" so we don't look
stupid.  Re-assigning to Ben.
Assignee: matt → ben
Keywords: correctness
Priority: P3 → P2
Whiteboard: [rtm+]

Comment 7

18 years ago
Actually I disagree with Blake here. People who use menus frequently expect the 
`Undo' and `Redo'/`Repeat' items to consistently be at the start of the Edit 
menu; and Cut, Copy, etc to be after them.

If you get rid of the Undo item, then you could have someone who intended to undo 
a Cut, instinctively selecting the first item in the Edit menu and accidentally 
doing another cut instead -- losing their first cut permanently! Or when Undo is 
implemented and the menu item returned, you could get people doing an undo when 
they intended to do a cut (which wouldn't be as bad as the first scenario, but 
would still be annoying).

I suggest changing the wording of the item to `Can't Undo' (as is standard when 
undo is not possible), but *don't* get rid of it.
OS: Windows 98 → All
Hardware: PC → All
(Reporter)

Comment 8

18 years ago
Sorry, disagree.

(1) People can read.  Most won't just assume the first item on the list is Undo 
without at least glancing at it first to see that it says `Cut'.  Furthermore, 
every app doesn't have Undo first (or at all).  Right now I'm looking at IE's 
Edit menu, and Cut is the first item (there's no Undo/Redo menuitems, even 
though the accelerator keys work).

(2) I don't understand your logic.  If we leave this item here, people are more 
likely to do daring operations, since they'll think they can just Undo them if 
they need to.  Even if it says Can't Undo, they'll just assume that it'll 
switch to say "Undo" after they do some operation.  If the item isn't there, 
people will be less likely to assume that they can Undo operations, and will be 
more careful.

(3) `Can't Undo' typically suggests that an Undo isn't possible at the given 
time -- not that it's never possible.

Comment 9

18 years ago
Standard UI guideline:  the "Edit" menu should start with "Undo" (which is 
disabled if it isn't functional.)
I'm with Blake.
Status: NEW → ASSIGNED

Comment 12

18 years ago
I agree with blake, especially argument (2).

Comment 13

18 years ago
Macintosh HIGs say yes: `All applications should support the Undo ... command[] 
... These commands provide standard text-editing abilities, which need to be 
available in modal dialog boxes such as the Save As dialog box [or the Bookmark 
Properties dialog!], *even though your application itself may not handle these 
features*' [emphasis added].

Windows UI guidelines say no: `Include general-purpose editing commands on the 
Edit menu ... [including] the Cut, Copy, and Paste transfer commands ... and the 
following commands (*if they are supported*) ... Undo ...' [emphasis added].

Communicator 4.x says yes: in 4.x's Search Messages dialog there is `Undo' in the 
Edit menu, even though you can never undo anything.

IE for Windows says no -- it doesn't have Undo in the Edit menu for the browser 
window, even though it *does* support undo in this situation -- you have to 
select it from the context menu, which is perverse. (Actually, I think this is an 
example of simplification gone mad.)

All other software on Mac OS says yes: in the View Options dialog, the Appearance 
control panel, the Control Strip control panel, the Keyboard control panel etc, 
Undo is present in the Edit menu even though it can never be used. In fact, I 
can't find any Mac software which *doesn't* ever have Undo in its Edit menu. 
Usually this is because Undo is actually implemented, but not always. I guess the 
consistency of position of the other menu items is given more weight than the 
possibility that the user might do something which they mistakenly thought they 
could undo.

Comment 15

18 years ago
Wow. So we have two specs.

I guess this means that on windows the menu should not be visible and on mac os 
it should say can't undo.  I'm guessing Linux falls with Windows.

mpt do you have time to research BeOS and other os's designed w/ UE in mind?

relnotertm: "Undo is not supported in manage bookmarks in this version of 
Netscape 6"
Keywords: relnoteRTM

Comment 16

18 years ago
The KDE interface guidelines say `Menu items should not be added or removed 
during runtime', but they don't say if this applies across components. The BeOS 
interface guidelines are very nearly non-existent, and don't mention menus at all 
yet.

My gut feeling is that you should leave the item there, but that's maybe just 
because I'm a Mac person.

Updated

18 years ago
Keywords: relnoteRTM

Comment 17

18 years ago
I give my strongest possible support to this bug. I believe a more careful reading of the
guidelines(even just the parts cited here) will demonstrate that in no way is it advocating
including a menuitem that will always, always be dead. Say that out loud and see if it makes
sense.

A disabled item implies that it will be enabled when appropriate - that's the whole entire point.
Otherwise the menuitem would not be  there! I'd love to hear someone explain the presence of
that menuitem to a novice!
Do not trick users and give them false hope. Also do not make them feel dumb when they can
never get it to activate.
'when appropriate' probably means 6.5, as lame as that is ;) seeking review for 
patch. 
Keywords: review

Comment 19

18 years ago
PDT marking [rtm need info] since no code review available yet. We agree the
menu item should be removed.
Whiteboard: [rtm+] → [rtm need info]
Hey Matt, gramps suggested I give this one to you as well. Pass it back to me if
you don't think you'll be able to get to it.

Thanks!
Assignee: ben → matt
Status: ASSIGNED → NEW
(Reporter)

Comment 21

18 years ago
Just need to check this in, right?  will do tomorrow.
Assignee: matt → blakeross
Ah, no.  This isn't marked "rtm++".  Also, at least on the Mac, the "Undo" menu 
item should be there and be disabled.  The idea was to use a platform overlay 
for this.  Bumping this bug back to Matt.
Assignee: blakeross → matt
(Reporter)

Comment 23

18 years ago
er...ok.  It's not marked rtm++ because the patch wasn't reviewed when the PDT 
came across this (see their comment).  I see no mention of using a platform 
overlay to keep this item on mac (albeit disabled), just some suggestions from 
Matthew that this remain on mac (but no real decision about what to do).

Still, unless Matt has some strong devotion to this bug, I can fix it....
Assignee: matt → blakeross
(Reporter)

Comment 24

18 years ago
Accepting bug.

If I can't get to this by the end of tomorrow, I'll bump back to matt..
Status: NEW → ASSIGNED
Don't check in the patch to remove the menuitem.  Feel free to come up with a 
more appropriate that uses overlays and attach it to this bug if you want... 
until then, let's leave this on Matt's plate.

(I don't want to play bug reassign games;  if you really want this bug and don't 
agree with me [the bookmark's module owner], assign it to Don... I talked with 
him yesterday in person, and am willing to do it again tomorrow.)
Assignee: blakeross → matt
Status: ASSIGNED → NEW
(Reporter)

Comment 26

18 years ago
<sigh>

OK.  Surely Matt has more important things on his plate than creating overlays 
to hide a menuitem.  And I didn't say I would check in the menuitem patch.

Looking forward to the patch Matt will have by tomorrow...

Comment 27

18 years ago
Adding to cc ...

Comment 28

18 years ago
approved for localization, please do the change asap (no later than next week)
(Assignee)

Comment 29

18 years ago
Well i'm happy to take this bug but I refuse to fix this with a overlay.
Either it's going to be disabled on all the platforms or it's going to be 
commented out on all the platforms.  If the mac guys need this menu item
disable then i see no reason why Unix and Windows should differ. 

I really don't care which way it's going to be fixed.
German and Ben own the UI from what i understand.  Can one of them make
a call?
Actually, I agree that an overlay is nasty.  How about just some JavaScript in 
bookmarks.js which checks the platform and, if its Mac, makes sure the menu item 
is disabled;  if its not the Mac, then just hide/remove/whatever the menu item?

Comment 31

18 years ago
Matt, if it's too much work to special case for one platform, then I would
prefer it removed on all platforms, not disabled.

Robert, do you want to take this and implement the special "disabled" case JS
for Mac?
Since most of the market is in a platform where having disabled items that don't 
do anything is insane, removing the item would be the statistically correct 
thing to do. (with a small amount of effort associated)

To satisfy rjc however merely requires closing this bug as WONTFIX. (almost no 
effort required).

I recommend the former, and suggest that a Mac person implements whatever UI is 
appropriate for that platform. ;)
Since I KNOW everyone here cares about the Mac just as much as every other 
platform we support (and yes, the Mac IS a platform we all care about, right?), 
here's some pseudo-code:

-----

// All Mac apps have an Undo menu item under the Edit menu, even if its disabled;
// for non Mac platforms, just remove the Undo menuitem (until its implemented)
if(navigator.appVersion.indexOf("Macintosh") < 0)
{
  undoItem = document.getElementById("menu_undo");
  parentNode = undoItem.parentNode;
  parentNode.removeChild(undoItem);
}

Note: you might also want to remove the next menu separator after the Undo node, 
but I'll leave as an exercise to the platform-caring reader.

Comment 34

18 years ago
jsconsole has an edit menu w/o an undo. Just comment out the menu (or add the 
hidden="true" param) . Yes we upset the apple cart for one revision of 
netscape6 but we already broke that rule. BeConsistent.

instead of calling remove if we do decide to create this menuitem macos should 
just remove thee hidden attribute.

cc{mac users}
if you have objections feel free to write the javascript for this :-(
(Assignee)

Comment 35

18 years ago
Created attachment 16733 [details] [diff] [review]
bookmarks changes to make the menu disappear
(Assignee)

Comment 36

18 years ago
Alright,

i made the patch to make the menu dissappear for all other programs
then the mac as well as the sepertator.  I haven't tested on the mac
to see if it works but i think rjc is volunteer since he wants this change in.
I need this tested and reviewed by tomorrow or we should just comment out the 
menu.  Ben and german thought we should do this anyways but this is my olive 
leaf attempt.
(Reporter)

Comment 37

18 years ago
hmm...Matt, I appreciate you making the patch, and it looks good, but now that 
PDT is only ++'ing P1 crash and dataloss bugs, this might not even be necessary 
anymore...
Tested on a Mac, works fine;  r=rjc

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 39

18 years ago
a=hyatt

Comment 40

18 years ago
PDT, please approve so we can remove the offending non-working command.
Whiteboard: [rtm need info] → [rtm+]Fix in hand, reviewed and approved

Comment 41

18 years ago
[rtm-] time to focus on crash and data loss bugs
Whiteboard: [rtm+]Fix in hand, reviewed and approved → [rtm-]Fix in hand, reviewed and approved
(Reporter)

Comment 42

18 years ago
well...if that's really the case, then this may as well be marked wontfix, 
since this was a fix for the branch only.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 43

18 years ago
Glad i spent the time getting that done.
(Reporter)

Comment 44

18 years ago
Sorry, Matt...
Status: RESOLVED → VERIFIED
Relnoting that this menu item is non-functional.

Gerv
Keywords: relnote
Whiteboard: [rtm-]Fix in hand, reviewed and approved → relnote-user [rtm-]Fix in hand, reviewed and approved
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.