Merge GetIsDocumentEditable implementation to EditorBase

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
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 hidden (mozreview-request)
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+

Comment 3

2 years ago
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/448a2d9d62c2
Merge GetIsDocumentEditable implementation to EditorBase. r=masayuki

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/448a2d9d62c2
Status: NEW → RESOLVED
Closed: 2 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.