123981 - Add missing editor embedding functionality in MfcEmbed...

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Chak Nanga]

Following in the footsteps of changes made to fix
http://bugzilla.mozilla.org/show_bug.cgi?id=123570...

here are some additional issues to be addressed wrt editor embedding in MfcEmbed:

1. Update toolbar buttons to show currently selected styles. For ex., show the
"bold" toolbar button as pressed when the user selects "bold"

2. Hook up the italic/underline toolbar buttons to do what the user expects them
to - currently they're not implemented

3. Get rid of code that's not needed or not doing anything right now.
Specifically, the CCommandObserver.* files. I'm guessing that this was added to
do something with updating the toolbar buttons - but it seems to do nothing.
Can't i just update the toolbar buttons by using
nsICommandManager->GetCommandState() to determine the command state and update
the buttons accordingly?

4. Currently there are menu items which do not make sense when in the editor
window . We need to fix up the menus appropriately.

5. When in the editor window, File/Save As.. does nothing now. How do i make it
save what i've entered?

6. When in the editor window, File/Open.. opens the file, but, not in edit mode.
How do i make it open in edit mode?


I know how to take care of items 1-4. Need your help in taking care of 5 and 6.

Comment 1

[Chak Nanga]

*** Bug 123982 has been marked as a duplicate of this bug. ***

Comment 2

[Chak Nanga]

Attachment: Patch to take care of items 1-4 in the initial bug report

Mike/Saari/Simon : 

Could you please r= the patch when you get a chance? Also, could you please
take a look at |CEditorFrame::UpdateStyleToolBarBtn()| - specifically the usage
of STATE_ALL/STATE_MIXED to update the toolbar buttons. This code seems to work
OK - but want to make sure that i'm doing it correctly....

Thanks

Comment 3

[Simon Fraser [no longer active]]

+#define BOLD_COMMAND        NS_ConvertASCIItoUCS2("cmd_bold")

This is a really bad way to declare constant wide strings. NS_LITERAL_STRING is 
what you want. Otherwise, things look good.

Comment 4

[Chak Nanga]

Hi Simon : I'll take care of #define issue with the wide strings. Thanks for
taking a look.

Comment 5

[Adam Lock]

Comment on attachment 68398 [details] [diff] [review]
Patch to take care of items 1-4 in the initial bug report

r=adamlock

Comment 6

[Chak Nanga]

Hi Simon : Can you please sr= this...thanks

Comment 7

[Simon Fraser [no longer active]]

Are you going to attach a new patch for the string issues?

+    params->SetBooleanValue(STATE_ALL, true);
+    params->SetStringValue(COMMAND_NAME, aCommand);
+    return DoCommand(params);

This is more an API issue. In my original work, DoCommand took 2 params:

DoCommand(in wstring commandName, in nsICommandParams params);

mjudge changed that to just take an nsICommandParams, setting the command name as 
an attribute on those params. I don't like this; it means that you can't call 
DoComand with a null nsICommandParams*, which you might want to do for simple 
toggle-style commands, or to indicate special behaviour ("throw UI"). I'd like to 
see it changed back. (This is work mjudge would have to do). 

Comment 8

[Chak Nanga]

Attachment: Updated patch with the NS_LITERAL_STRING changes

Hi Simon:

Wrt to the DoCommand() issue i'll file a new bug on mjudge. 

Meanwhile, i'd like to get this checked in so that we have something a bit more
usable.
Thanks

Comment 9

[Simon Fraser [no longer active]]

Comment on attachment 68590 [details] [diff] [review]
Updated patch with the NS_LITERAL_STRING changes

sr=sfraser

Comment 10

[Chak Nanga]

Fix checked in.

Adam/Simon : Thank you for the r=/sr='ing the patches....

Comment 11

[Michael Dunn]

Clean up verification of dated code change bus