1801028 - Make style editor methods which take HTML property and attribute separately take EditorInlineStyle instead

Status: RESOLVED FIXED

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

Description

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

I'm now afraid to touch the logic in CSSEditUtils because an user sets HTML property to nullptr and attribute to nsGkAtoms::align.
https://searchfox.org/mozilla-central/rev/d7d2cc647772de15c4c5aa47f74d25d0e379e404/editor/libeditor/HTMLEditorState.cpp#346

For avoiding regressions, I think that we should make all of them take EditorInlineStyle or EditorInlineStyleAndValue to check the relation at the root callers. Then, perhaps, they should be renamed to EditorElementStyle and EditorElementStyleAndValue since only setting attribute works as generic attribute change of any elements.

Comment 1

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

Attachment: Bug 1801028 - part 1: Make `HTMLEditor::IsRemovableParentStyleWithNewSpanElement` take `EditorInlineStyle` r=m_kato!

Comment 2

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

Attachment: Bug 1801028 - part 2: Move `CSSEditUtils::IsCSSInvertible` to `EditorInlineStyle` r=m_kato!

It does not make sense to make the static method take only EditorInlineStyle.

Depends on D162501

Comment 3

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

Attachment: Bug 1801028 - part 3: Make `HTMLEditor::GetInlinePropertyBase` take `EditorInlineStyle` r=m_kato!

Depends on D162502

Comment 4

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

Attachment: Bug 1801028 - part 4: Make `HTMLEditor::ClearStyleAt` take `EditorInlineStyle` r=m_kato!

Depends on D162503

Comment 5

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

Attachment: Bug 1801028 - part 5: Make `HTMLEditor::SplitAncestorStyledInlineElementsAtRangeEdges` take `EditorInlineStyle` r=m_kato!

Depends on D162504

Comment 6

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

Attachment: Bug 1801028 - part 6: Make `HTMLEditor::SplitAncestorStyledInlineElementsAt` take `EditorInlineStyle` r=m_kato!

Depends on D162505

Comment 7

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

Attachment: Bug 1801028 - part 7: Make `HTMLEditor::SetInlinePropertyOnTextNode` take `EditorInlineStyleAndValue` r=m_kato!

Depends on D162506

Comment 8

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

Attachment: Bug 1801028 - part 8: Make `HTMLEditor::SetInlinePropertyOnNode` and `HTMLEditor::SetInlinePropertyOnNodeImpl` take `EditorInlineStyleAndValue` r=m_kato!

They are called each other. Therefore, this patch changes them once.

Depends on D162507

Comment 9

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

Attachment: Bug 1801028 - part 9: Make `HTMLEditor::ElementIsGoodContainerForTheStyle` take `EditorInlineStyleAndValue` r=m_kato!

Depends on D162508

Comment 10

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

Attachment: Bug 1801028 - part 10: Make `HTMLEditor::RemoveStyleInside` take `EditorInlineStyle` r=m_kato!

Depends on D162509

Comment 11

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

Attachment: Bug 1801028 - part 11: Make `CSSEditUtils::Is(Computed|Specified)CSSEquivalentToHTMLInlineStyleSet` take `EditorInlineStyle` r=m_kato!

Depends on D162510

Comment 12

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

Attachment: Bug 1801028 - part 12: Make `CSSEditUtils::Have(Specified|Computed)CSSEquivalentStyles` take `EditorInlineStyle` r=m_kato!

Depends on D162511

Comment 13

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

Attachment: Bug 1801028 - part 13: Make `HTMLEditUtils::IsInlineStyleSetByElement` take `EditorInlineStyle` r=m_kato!

Depends on D162512

Comment 14

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

Attachment: Bug 1801028 - part 14: Make `CSSEditUtils::SetCSSEquivalentToHTMLStyle(With|Without)Transaction` take `EditorElementStyle` r=m_kato!

This patch creates a base class of EditorInlineStyle, the name is
EditorElementStyle. It requires only attribute of HTML. However, it's
obviously different from EditorInlineStyle's rule which is mHTMLProperty
cannot be nullptr. Therefore, the methods which can treat only
EditorInlineStyle do not want the data only whose attribute is not nullptr.
For solving this issue at build time, this approach is better than renaming
EditorInlineStyle and make it have the new mode.

Depends on D162513

Comment 15

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

Attachment: Bug 1801028 - part 15: Make `HTMLEditor::RemoveCSSEquivalentToHTMLStyle(With|Without)Transaction` take `EditorElementStyle` r=m_kato!

Depends on D162514

Comment 16

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

Attachment: Bug 1801028 - part 16: Make `CSSEditUtils::GetComputedCSSEquivalentToHTMLInlineStyleSet` take `EditorElementStyle` r=m_kato!

Depends on D162515

Comment 17

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

Attachment: Bug 1801028 - part 17: Make `CSSEditUtils::IsCSSEditableProperty` take `EditorElementStyle` r=m_kato!

It can take const Element& instead of nsINode*, then most callers can be
simpler.

Depends on D162516

Comment 18

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

Attachment: Bug 1801028 - part 18: Make `CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle` take `EditorElementStyle` r=m_kato!

And also making it and BuildCSSDeclarations return pairs of CSS property and
value with an array makes their callers simpler.

Depends on D162517

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6123f16df995
part 1: Make `HTMLEditor::IsRemovableParentStyleWithNewSpanElement` take `EditorInlineStyle` r=m_kato

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c53e1725fcd7
part 2: Move `CSSEditUtils::IsCSSInvertible` to `EditorInlineStyle` r=m_kato

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43809c02bf7e
part 3: Make `HTMLEditor::GetInlinePropertyBase` take `EditorInlineStyle` r=m_kato

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cec746f53e3f
part 4: Make `HTMLEditor::ClearStyleAt` take `EditorInlineStyle` r=m_kato

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6423b58e19d9
part 5: Make `HTMLEditor::SplitAncestorStyledInlineElementsAtRangeEdges` take `EditorInlineStyle` r=m_kato

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80add9777122
part 6: Make `HTMLEditor::SplitAncestorStyledInlineElementsAt` take `EditorInlineStyle` r=m_kato

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e30ed4ad5e7e
part 7: Make `HTMLEditor::SetInlinePropertyOnTextNode` take `EditorInlineStyleAndValue` r=m_kato

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87a1b8d813d2
part 8: Make `HTMLEditor::SetInlinePropertyOnNode` and `HTMLEditor::SetInlinePropertyOnNodeImpl` take `EditorInlineStyleAndValue` r=m_kato

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98fc9276e73c
part 9: Make `HTMLEditor::ElementIsGoodContainerForTheStyle` take `EditorInlineStyleAndValue` r=m_kato

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/883bf0a6d10e
part 10: Make `HTMLEditor::RemoveStyleInside` take `EditorInlineStyle` r=m_kato

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9252b81cc71c
part 11: Make `CSSEditUtils::Is(Computed|Specified)CSSEquivalentToHTMLInlineStyleSet` take `EditorInlineStyle` r=m_kato

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6224033594d
part 12: Make `CSSEditUtils::Have(Specified|Computed)CSSEquivalentStyles` take `EditorInlineStyle` r=m_kato

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f790bc14ced
part 13: Make `HTMLEditUtils::IsInlineStyleSetByElement` take `EditorInlineStyle` r=m_kato

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b20755464664
part 14: Make `CSSEditUtils::SetCSSEquivalentToHTMLStyle(With|Without)Transaction` take `EditorElementStyle` r=m_kato

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f07e20564d4
part 15: Make `HTMLEditor::RemoveCSSEquivalentToHTMLStyle(With|Without)Transaction` take `EditorElementStyle` r=m_kato

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e59040efaf3b
part 16: Make `CSSEditUtils::GetComputedCSSEquivalentToHTMLInlineStyleSet` take `EditorElementStyle` r=m_kato

Comment 36

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/6123f16df995
https://hg.mozilla.org/mozilla-central/rev/c53e1725fcd7
https://hg.mozilla.org/mozilla-central/rev/43809c02bf7e
https://hg.mozilla.org/mozilla-central/rev/cec746f53e3f
https://hg.mozilla.org/mozilla-central/rev/6423b58e19d9
https://hg.mozilla.org/mozilla-central/rev/80add9777122
https://hg.mozilla.org/mozilla-central/rev/e30ed4ad5e7e
https://hg.mozilla.org/mozilla-central/rev/87a1b8d813d2
https://hg.mozilla.org/mozilla-central/rev/98fc9276e73c

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13da38d1f15e
part 17: Make `CSSEditUtils::IsCSSEditableProperty` take `EditorElementStyle` r=m_kato

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59d691febda1
part 18: Make `CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle` take `EditorElementStyle` r=m_kato

Comment 39

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/883bf0a6d10e
https://hg.mozilla.org/mozilla-central/rev/9252b81cc71c
https://hg.mozilla.org/mozilla-central/rev/d6224033594d
https://hg.mozilla.org/mozilla-central/rev/4f790bc14ced
https://hg.mozilla.org/mozilla-central/rev/b20755464664
https://hg.mozilla.org/mozilla-central/rev/4f07e20564d4
https://hg.mozilla.org/mozilla-central/rev/e59040efaf3b
https://hg.mozilla.org/mozilla-central/rev/13da38d1f15e
https://hg.mozilla.org/mozilla-central/rev/59d691febda1