1336349 - Merge GetIsDocumentEditable implementation to EditorBase

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Makoto Kato [:m_kato]]

GetIsDocumentEditable is implemnted in EditorBase, TextEditor, and HTMLEditor.  This is virtual method, we won't use EditorBase::GetIsDocumentEditable.  Also, TextEditor::GetIsDocumentEditable and HTMLEditor::GetIsDocumentEditable are same implementation.  So we should merge this to EditorBase.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1336349 - Merge GetIsDocumentEditable implementation to EditorBase.

GetIsDocumentEditable is implemnted in EditorBase, TextEditor, and HTMLEditor.  This is virtual method, we won't use EditorBase::GetIsDocumentEditable.  Also, TextEditor::GetIsDocumentEditable and HTMLEditor::GetIsDocumentEditable are same implementation.  So we should merge this to EditorBase.

Review commit: https://reviewboard.mozilla.org/r/109444/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109444/

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8833215 [details]
Bug 1336349 - Merge GetIsDocumentEditable implementation to EditorBase.

https://reviewboard.mozilla.org/r/109444/#review110528

::: editor/libeditor/EditorBase.h:860
(Diff revision 1)
> +  bool IsModifiable() const
> +  {
> +    return !IsReadonly();
> +  }

Hmm, in editor/libeditor, "editable" and "modifiable" are mixed...

https://dxr.mozilla.org/mozilla-central/search?q=Modifiable+path%3Aeditor%2Flibeditor&redirect=false
https://dxr.mozilla.org/mozilla-central/search?q=Editable+path%3Aeditor%2Flibeditor&redirect=false

Although, this is out of scope of this bug.

::: editor/libeditor/EditorBase.cpp:548
(Diff revision 1)
>  NS_IMETHODIMP
>  EditorBase::GetIsDocumentEditable(bool* aIsDocumentEditable)
>  {
>    NS_ENSURE_ARG_POINTER(aIsDocumentEditable);
>    nsCOMPtr<nsIDocument> doc = GetDocument();
> -  *aIsDocumentEditable = !!doc;
> +  *aIsDocumentEditable = doc && IsModifiable();

IsModifiable() shouldn't be false if this is an HTML editor (but possible case, though).

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/448a2d9d62c2
Merge GetIsDocumentEditable implementation to EditorBase. r=masayuki

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/448a2d9d62c2