184719 - midas needs a command for toggling read-only/writable

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Kathleen :Brade]

Midas needs a command for toggling read-only and writeable modes

Comment 1

[Kathleen :Brade]

Charley reminds me of these things:
>be sure to implement it as another "nsSetDocumentStateCommand" command if it
>supports setting a state param. and use nsDocumentStateCommand if it simply
>reports state

I would guess that this is setting a state rather than reporting a state.

Comment 2

[Kathleen :Brade]

Attachment: split document commands off into a separate file; add readonly cmd

Comment 3

[Kathleen :Brade]

Comment on attachment 111432 [details] [diff] [review]
split document commands off into a separate file; add readonly cmd

sfraser--can you look this over and give me some feedback?  Note: much of the
new file is copy/pasted from old file so some cleanup may be needed.

Comment 4

[Simon Fraser [no longer active]]

+    // Should we fail if this param wasn't set?
+    // I'm not sure we should be that strict
+    if (NS_FAILED(rv))
+      return NS_OK;

I think if you're going to not do what the user asked, then you should return an
error value.

+    if (NS_SUCCEEDED(aParams->GetBooleanValue(STATE_ATTRIBUTE, &isReadOnly)))
+    {
+      PRUint32 flags;
+      editor->GetFlags(&flags);
+      if (isReadOnly)
+        flags |= nsIPlaintextEditor::eEditorReadonlyMask;
+      else
+        flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
+
+      return NS_OK;
+    }
+
+    return NS_ERROR_FAILURE;

Don't you need to set the flags back on the editor? An early return would be
cleaner here too (return failure if GetBooleanValue() fails).

+  {
+
+    PRUint32 editorStatus = nsIEditingSession::eEditorErrorUnknown;

extra line.

+  nsCOMPtr<nsIEditingSession> editingSession = do_QueryInterface(refCon);

Need to say why the refcon for this command is an editingSession, and for the
other commands is an nsIEditor (see below).

+      // refCon is initially set to nsIEditingSession until editor
+      //  is successfully created and source doc is loaded

Oh, there it is. Move the
+  nsCOMPtr<nsIEditingSession> editingSession = do_QueryInterface(refCon);
inside that scope.

Comment 5

[Kathleen :Brade]

Attachment: revised diff with fixes suggested by smfr

Comment 6

[Kathleen :Brade]

Comment on attachment 111627 [details] [diff] [review]
revised diff with fixes suggested by smfr

mkaply--please review.

note: commented out ifdefs will be removed once I've confirmed they build on
windows/linux.

Comment 7

[Mike Kaply [:mkaply]]

Comment on attachment 111627 [details] [diff] [review]
revised diff with fixes suggested by smfr

fyi this builds on windows with a Makefile.in change

Comment 8

[Kathleen :Brade]

Attachment: add command registration

obsoleting previous patch because it was checked in

Comment 9

[Kathleen :Brade]

Comment on attachment 111792 [details] [diff] [review]
add command registration

seek reviews for missing command registration

Comment 10

[Kathleen :Brade]

Comment on attachment 111792 [details] [diff] [review]
add command registration

obsoleting patch which is now checked in

Comment 11

[Kathleen :Brade]

fixed