41735 - Checkmarks in Composer menus are missing -- DL commands not working

Status: VERIFIED FIXED

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

Description

[Charles Manske]

There are many places in our menus which are supposed to show a checkmark
when the menuitem has the attribute "checked='true'".
None of them are currently showing the checkmark. The most globabl example
is to look in the browser's View | Toolbars submenu: The toolbar items that 
are visible should have a checkmark in front of them. 
There's other examples in Format submenus as well.
Here's where it gets weird: The checkmarks show 100% correctly on Mac,
they work in the Browser, but not Composer

Comment 1

[Charles Manske]

Hyatt helped out and the fix is trivial. All menuitems that support a checkmark
must have the attribute: type="checkbox" on the <menuitem>.
The checkmark feedback is very important to editor useability, so I'm
nominating for nsbeta+ and dogfood.
Mac doesn't show the problem since they use native menus and don't need the
"checkbox" attribute.

Comment 2

[Jim Roskind]

Putting on beta2 plus radar since value is high, and fix is trivial.
Thanks,

Jim

Comment 3

[Charles Manske]

checked in today.

Comment 4

[sujay]

still not fixed in 6/9 build...the only checkmark I see is under View menu for
sidebar.

Comment 5

[Charles Manske]

Works in my debug build. Please test with later build, I guess.

Comment 6

[sujay]

go ahead and mark this fixed....I'll re-verify with later build...

Comment 7

[Kathleen :Brade]

hmmm... I don't think this is fixed.  If you want me to file a separate bug, 
please say so.

With a Mac debug build from today, I am seeing that checkmarks don't go away when 
I would expect them to.  In the heading menu, I can change some text to be a 
heading and that works fine.  However, if I change my mind and pick a different 
heading, I see a checkmark on the old item as well as the new item.  If I pick 
normal, the behavior is as expected but now I get 3 checkmarks.  Selecting these 
items removes the checkmarks but sometimes the state is messed up and the 
checkmarks should be there and aren't.  I'd rather not have checkmarks at all 
than have them show up and work wrong.

Comment 8

[Charles Manske]

Kathy is correct - checkmarks on previously-set headings are not removed.
This didn't happen last week when first implemented. 
I'm soooo tired of regressions (not in editor code)!

Comment 9

[Charles Manske]

In trying to fix the checkmark problem, I'm finding that the "Term" and 
"Definition" menuitems do not work intermitantly, e.g., when used after
selecting a bullet or number list item first. This is a more serious 
problem and should be fixed for beta2. Requesting extension to 6/22 to 
get all the items in the list submenu working correctly.

Comment 10

[Charles Manske]

Fix for the basic problem of setting checkmarks is ready and reviewed, but
tree is not opening any time soon.
There are 2 problems that I will file separate bugs to address after beta2:
1. Sometimes commands don't fire (Heading, Paragraph, and List menu items 
   tested)
2. Blockquote should not be treated as a "paragraph style", but should be 
   separated out. A paragraph can have a particular style, e.g., a heading,
   *and* have "blockquote" style as a parent block tag. By mixing these
   into the same menu, the blockquote masks any child paragraph styles
   (nsHTMLEditor::GetParagraphStyle() returns "blockquote" instead of child
    style.)

Comment 11

[Charles Manske]

Checked in. Should be working (except for problems noted above) in today's
build.

Comment 12

[sujay]

verified in 6/16 build.