79200 - unable to remove "fixed width font"

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Kathleen :Brade]

I am unable to remove the "fixed width" font setting as I expect.

* Create a new Composer document.
* type some text
* press accel-shift-T (or choose fixed width from Format | Font menu)
* type some more text
* press accel-shift-T or choose the fixed width menu item again to turn off
* type some more text

Notice that the most recent text is still fixed width.
I expect it to work like the bold and italic menu items.

Comment 1

[rubydoo123]

moving to 1.0

Comment 2

[Charles Manske]

Darn! It doesn't act like a toggle. This would be easy to fix, but is a lower
priority compared to other bugs.
The workaround is to use the keybinding to "Discontinue Text Styles", which is
accel+shift+T
Note that the keybinding for setting "fixed-width" font is now accel+T.

Comment 3

[Charles Manske]

changing milestone to 0.9.4

Comment 4

[Charles Manske]

The fix is to detect our current state and toggle the "TT" (fixed width)
inline style. There are two approaches to use for the new method:
1. Manipulate the command node:
function toggleFixedWidthFont()
{
  var commandNode = document.getElementById("cmd_fontFace");
  if (commandNode)
  {
    var state = commandNode.getAttribute("state");

    commandNode.setAttribute("state", state == "tt" ? "" : "tt");

    gContentWindow.focus();
    goDoCommand("cmd_fontFace");
  }
}

2. Use more direct editorShell methods:
function toggleFixedWidthFont()
{
  var firstHas = new Object;
  var anyHas = new Object;
  var allHas = new Object;
  allHas.value = false;

  window.editorShell.GetTextProperty("tt", "", "", firstHas, anyHas, allHas);

  if (allHas.value)
    window.editorShell.RemoveTextProperty("tt", "");
  else
    window.editorShell.SetTextProperty("tt", "", "");

  gContentWindow.focus();
  goUpdateCommand("cmd_fontFace");
}
Which of these seems better?

Both work (or don't work) exactly the same. There's a problem with removing
the "TT" tag when selection spans 2 contiguous stretches of TT like this:
<tt>1111</tt><tt>2222</tt>
I'll investigate if this is a known problem and file bugs if needed.

Comment 5

[Simon Fraser [no longer active]]

We should fix this in a such a way that _if_ we added a 'TT' button to the 
toolbar (like Bold, Italics), it would "just work", and reflect state. So neither 
of the above approaches seem ideal.

Comment 6

[Charles Manske]

We decided a long time ago that it was better UI to treat "TT" as a font and 
not as a text style. But for "power users", availability of the old standard 
Ctrl+T toggle keybinding for fixed width type seemed like a good idea.
Full support of this would involve duplicating the command code of other 
text styles such as bold. I'm not sure if that's worth the effort, but if we 
decide to do it it should be in 0.9.4.
I suggest using version '1' (using command nodes) as a fix for 0.9.4. 
It seems easier to convert if we decide to add full support for TT as text 
attribute.

Comment 7

[Charles Manske]

Attachment: Make accel+T keybinding act like a toggle.

Comment 8

[Kathleen :Brade]

I don't like that fix.

Comment 9

[Charles Manske]

Attachment: Add a "Fixed Width" command just like other text styles

Comment 10

[Charles Manske]

Note that with the 8/21/01 patch, a "cmd_tt" is implemented and the
"toggleFixedWidth()" method that already existed in editor.js is no longer
necessary, so I removed it.
Setting any text style via the menuitems doesn't work currently (has nothing to
do with this fix.) I'll find or file a bug on that issue.

Comment 11

[Kathleen :Brade]

r=brade

Comment 12

[Simon Fraser [no longer active]]

sr=sfraser

Comment 13

[Charles Manske]

checked in

Comment 14

[Simon Fraser [no longer active]]

The 'Fixed width' menu item is permanently disabled for me, and some debug dumps 
are showing 'No controller found for command cmd_tt'. This ain't right.

Comment 15

[Simon Fraser [no longer active]]

it seems that you forgot to check in nsComposerController.cpp.

Comment 16

[Charles Manske]

Oops! Missed checking in nsComposerController.cpp

Comment 17

[Asa Dotzler [:asa]]

a=asa on behalf of drivers.

Comment 18

[Charles Manske]

nsComposerController now checked in.

Comment 19

[sujay]

verified.