Closed Bug 1442499 Opened 2 years ago Closed Last year

EditorBase::IsPreformatted() should return bool

Categories

(Core :: Editor, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(1 file)

To get nsStyleContext, it is unnecessary to use PresShell, so we can use bool as return type.
Assignee: nobody → m_kato
Comment on attachment 8968397 [details]
Bug 1442499 - EditorBase::IsPreformatted should return bool.

https://reviewboard.mozilla.org/r/237094/#review242872

::: editor/libeditor/EditorBase.cpp:4026
(Diff revision 1)
>   */
> -nsresult
> -EditorBase::IsPreformatted(nsIDOMNode* aNode,
> -                           bool* aResult)
> +// static
> +bool
> +EditorBase::IsPreformatted(nsINode* aNode)
>  {
> -  nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
> +  if (!aNode) {

NS_WARN_IF? Or, use nsINode& for aNode?

::: editor/libeditor/HTMLEditRules.cpp:5995
(Diff revision 1)
> -    bool isPRE;
> +    if (EditorBase::IsPreformatted(nextNode)) {
> -    htmlEditor->IsPreformatted(nextNode->AsDOMNode(), &isPRE);
> -    if (isPRE) {
>        if (EditorBase::IsTextNode(nextNode)) {
>          nsAutoString tempString;
>          nextNode->GetAsText()->GetData(tempString);
>          int32_t newlinePos = tempString.FindChar(nsCRT::LF);
>          if (newlinePos >= 0) {
>            if (static_cast<uint32_t>(newlinePos) + 1 == tempString.Length()) {
>              // No need for special processing if the newline is at the end.
>              break;
>            }
>            return EditorDOMPoint(nextNode, newlinePos + 1);
>          }
>        }
>      }

Could you connect those conditions with && and reduce the indent?
Attachment #8968397 - Flags: review?(masayuki) → review+
Comment on attachment 8968397 [details]
Bug 1442499 - EditorBase::IsPreformatted should return bool.

https://reviewboard.mozilla.org/r/237094/#review242984

::: editor/libeditor/EditorBase.cpp:4026
(Diff revision 1)
>   */
> -nsresult
> -EditorBase::IsPreformatted(nsIDOMNode* aNode,
> -                           bool* aResult)
> +// static
> +bool
> +EditorBase::IsPreformatted(nsINode* aNode)
>  {
> -  nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
> +  if (!aNode) {

I will use NS_WARN_IF
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/767f9e9dd582
EditorBase::IsPreformatted should return bool. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/767f9e9dd582
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.