1435149 - some methods in CSSEditUtils should be static method

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement, P3)

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1435149 - Part 1. Change some methods to static or const method in CSSEditUtils.

Some methods in CSSEditUtils can change to static methods or const method.
So we should change to it to avoid unnecessary HTMLEditor check.

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

Comment 2

[Makoto Kato [:m_kato]]

Attachment: Bug 1435149 - Part 2. Use scope resolution operator for CSSEditUtils's caller.

Part 1. changes to static method, so caller should use scope operator instead
of mCSSEditUtils member.

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

Comment 3

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

Comment on attachment 8947743 [details]
Bug 1435149 - Part 1. Change some methods to static or const method in CSSEditUtils.

https://reviewboard.mozilla.org/r/217442/#review223234

::: editor/libeditor/CSSEditUtils.h:234
(Diff revision 1)
> -  bool HaveCSSEquivalentStyles(nsINode& aNode,
> +  static bool HaveCSSEquivalentStyles(nsINode& aNode,
> -                               nsAtom* aProperty,
> +                                       nsAtom* aProperty,
> -                               nsAtom* aAttribute,
> +                                       nsAtom* aAttribute,
> -                               StyleType aStyleType);
> +                                       StyleType aStyleType);

nit: Looks you added one space redundantly. Please check it before landing.

::: editor/libeditor/CSSEditUtils.h:346
(Diff revision 1)
> -  already_AddRefed<nsComputedDOMStyle> GetComputedStyle(dom::Element* aElement);
> +  static already_AddRefed<nsComputedDOMStyle> GetComputedStyle(
> +                                                dom::Element* aElement);

According to other header files, in this case,

static already_AdRefed<nsComputedDOMStyle>
GetComputedStyle(dom::Element* aElement);

is usual style.

::: editor/libeditor/CSSEditUtils.h:373
(Diff revision 1)
> -  void BuildCSSDeclarations(nsTArray<nsAtom*>& aPropertyArray,
> +  static void BuildCSSDeclarations(nsTArray<nsAtom*>& aPropertyArray,
> -                            nsTArray<nsString>& cssValueArray,
> +                                    nsTArray<nsString>& cssValueArray,
> -                            const CSSEquivTable* aEquivTable,
> +                                    const CSSEquivTable* aEquivTable,
> -                            const nsAString* aValue,
> +                                    const nsAString* aValue,
> -                            bool aGetOrRemoveRequest);
> +                                    bool aGetOrRemoveRequest);

nit: Looks you added one more space here too.

Comment 4

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

Comment on attachment 8947744 [details]
Bug 1435149 - Part 2. Use scope resolution operator for CSSEditUtils's caller.

https://reviewboard.mozilla.org/r/217444/#review223236

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b11498239be6
Part 1. Change some methods to static or const method in CSSEditUtils. r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc2e682094fb
Part 2. Use scope resolution operator for CSSEditUtils's caller. r=masayuki

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b11498239be6
https://hg.mozilla.org/mozilla-central/rev/cc2e682094fb