Closed Bug 1336349 Opened 4 years ago Closed 4 years ago

Merge GetIsDocumentEditable implementation to EditorBase

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(1 file)

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 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).
Attachment #8833215 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/448a2d9d62c2
Merge GetIsDocumentEditable implementation to EditorBase. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/448a2d9d62c2
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee: nobody → m_kato
You need to log in before you can comment on or make changes to this bug.