137255 - Rewrap should be edit menuitem, not in option menu

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Boris 'pi' Piwinger]

Not limited to a certain Mozilla version. This is about the user interface of
the plain text editor.

Actual: Rewrap is in the options menu.

Expected: Rewrap is in the edit menu.

Rationale: Rewrap is a one time action to edit the mail (or news) text. As an
option it would have an effect on other actions which is not the case.

pi

Comment 1

[Boris 'pi' Piwinger]

Jean-Francois Ducarroz in http://bugzilla.mozilla.org/show_bug.cgi?id=147502#c4

who ever wants to work on those UI issues must consult with Jennifer Glick
([e-mail]) before waisting his/her time on a solution that could be
rejected.

So be it. -> CC

pi

Comment 2

[Boris 'pi' Piwinger]

Just learned, that Composition would be the right component. Sorry.

pi

Comment 3

[Boris 'pi' Piwinger]

Is there any chance, we get this in? Probably it is a simple fix. As in bug
147502 won't find it there. It is more than obvious that it is not an option,
since it only has a one-time-effect.

pi

Comment 4

[Boris 'pi' Piwinger]

I guess this bug is trivial to fix. Can we get it in for 1.2b, please?

pi

Comment 5

[Kathleen :Brade]

Jennifer--do you know the rationale for "Rewrap" being in the Option menu
instead of the "Edit" menu?

Comment 6

[jglick]

Where in the Edit menu would you liked to see it grouped?

It could also fit nicely in the Format menu.

Comment 7

[Boris 'pi' Piwinger]

I think it should be near to "paste as quotation", also "quote message" (bug
147502) would fit there.

So maybe:
- paste as quotation
- quote message
- rewrap
- delete

pi

Comment 8

[Jean-Francois Ducarroz]

-->varada

Comment 9

[Boris 'pi' Piwinger]

Jennifer, you agreed to fix it, do you also agree to the placement in comment 7?
Let's get this fixed now.

pi

Comment 10

[jglick]

- Paste as quotation
- Rewrap
- Delete

Ok with me.

Comment 11

[[e-mail]]

Attachment: Proposed patch

Comment 12

[Kathleen :Brade]

Comment on attachment 103318 [details] [diff] [review]
Proposed patch

this patch is fine with me but can you fix the editorshell usage to instead use
editor?  I worry that this patch will conflict with cmanske's work on
editorshell removal.

r=brade

Comment 13

[[e-mail]]

> I worry that this patch will conflict with cmanske's work

Um, it's just moving a menuitem, how will it conflict?

Comment 14

[Kathleen :Brade]

Comment on attachment 103318 [details] [diff] [review]
Proposed patch

I expect that Charley is touching this line for the last fragment:
  oncommand="editorShell.Rewrap(false)"

Since he has a large patch, I wouldn't expect him to realize that the menu item
has moved so he may not fix the menu item in the new location.

Comment 15

[[e-mail]]

Attachment: Better patch

1. Removed superfluous oncommand handler (uses command="cmd_rewrap")
2. Made disabled state update, so only enabled when content has focus
3. Changed accesskey from R to w

Comment 16

[Charles Manske]

Comment on attachment 103858 [details] [diff] [review]
Better patch

r=cmanske

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

>so only enabled when content has focus

why that? it seems to me that rewrap would work usefully even if content does
not have focus. (e.g. you're editing the subject, and realize you want the text
rewrapped... or did I misunderstand you, and that'd work?)

Comment 18

[Charles Manske]

Comment on attachment 103858 [details] [diff] [review]
Better patch

"backing out" my r=
It turns out this doesn't
work completely.
I am already messing with mail composer commands in the big "remove
editorShell" project (mailnews portion is bug 174389), so I'll take care  of
the problem correctly as part of that bug. I'll be sure to put menuitem in edit
menu as prescribed here.

Comment 19

[Charles Manske]

taking this bug

Comment 20

[Charles Manske]

Comment on attachment 103858 [details] [diff] [review]
Better patch

doh! Neil's patch works perfectly! I accidently deleted the "<command..." line
from the XUL!

Comment 21

[Charles Manske]

Fixed as part of checkin to bug 174389

Comment 22

[[e-mail]]

Because of the conflict with the "HTML rewrap" patch there are now two
cmd_rewrap cases in one of the switch statements :-(

Comment 23

[Boris 'pi' Piwinger]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021122

Verifying.

pi

Comment 24

[Mike Cowperthwaite]

I am confused whether this has actually been implemented as described in either
comment 7 or comment 10 (I prefer the solution in 7) -- either way, in version
1.2.1, the menus appear as they did in 1.1.

The discussion here makes it sound like the fix was handled in code; but if it's
(also?) an XUL issue, I'm using the Orbit theme, for whatever that's worth.

Comment 25

[Charles Manske]

In comment #10, jglick approved moving "Rewrap" into the Edit menu. 
The current version (from 1.3 trunk builds) should show this.
No mention was made of the "Quote message" item which remains in the Options menu.