Closed Bug 1781994 Opened 2 years ago Closed 2 years ago

Make inline property setter and remover stop touching `Selection` directly

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
105 Branch
Tracking Flags:
Tracking Status
firefox105 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

Attachments

(12 files)

Bug 1781994 - part 1: Make `HTMLEditor::SplitAncestorStyledInlineElementsAt` and `HTMLEditor::ClearStyleAt` stop touching `Selection` directly r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 2: Make `HTMLEditor::SplitAncestorStyledInlineElementsAtRangeEdges` stop touching `Selection` directly r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 3: Make `HTMLEditor::SetInlinePropertyOnTextNode` stop touching `Selection` directly r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 4: Make `HTMLEditor::RemoveInlinePropertyInternal` use `AutoRangeArray` r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 5: Make `HTMLEditor::RemoveInlinePropertyAsSubAction` handle correct text node after splitting a text to any direction r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 6: Make `HTMLEditor::SetInlinePropertyInternal` use `AutoRangeArray` r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 7: Make `HTMLEditor::RemoveInlinePropertyAsSubAction` handle multiple styles once r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 8: Make `HTMLEditor::SetInlinePropertyAsSubAction` handle multiple styles once r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 9: Make `HTMLEditor::RelativeFontChangeOnNode` and `HTMLEditor::RelativeFontChangeHelper` use `FontSize` r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 10: Make `HTMLEditor::RelativeFontChangeOnNode` and `HTMLEditor::RelativeFontChangeHelper` stop touching `Selection` directly r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 11: Clean up `HTMLEditor::RelativeFontChange` r=m_kato!
(deleted), text/x-phabricator-request
Details
Bug 1781994 - part 12: Make `HTMLEditor::SetCSSBackgroundColorWithTransaction` use similar approach as `SetInlinePropertiesAsSubAction` etc r=m_kato!
(deleted), text/x-phabricator-request
Details

Currently, it modifies Selection a lot and multiple updates of the DOM tree refers Selection, so the nested for loops in it is stateful and hard to understand.

I think that I can refactor it much simpler than now with the new helper classes and methods.

Summary: Redesign `HTMLEditor::RemoveInlinePropertyInternal` and its callees → Make inline property setter and remover stop touching `Selection` directly

I've already made a caller of HTMLEditor::ClearStyleAt,
HTMLEditor::CreateStyleForInsertText, in bug 1770877, so this fixes a bug of
the patch.

HTMLEditor::ClearStyleAt is still updates Selection only in some cases.
And one of the caller has not handle the Selection update. Therefore, once
it completely stop touching Selection, ComputeEditingHost will fail and
the other paths update Selection, so it should do it too. (Without the
change, test_dragdrop.html fails.)

Finally, we don't need EditResult anymore because we have
Result<EditorDOMPoint, nsresult>.

I'd like to get rid of AutoSelectionRangeArray and AutoRestoreSelection,
and I'd like to make it stop touching Selection while it is removing style
of each content nodes in the range. Therefore, this patch rewrites it with
AutoRangeArray. Then, we can reduce the indent level of the nested for
loops.

Note that it creates AutoEditSubActionNotifier, so it's a sub-edit action
handler. Therefore, this patch renames it.

Depends on D154345

This is a pre-fix for bug 1735608. Currently, the loop assumes that collected
nodes are always right node, but it'll be changed, therefore, it creates
new array for the follow up loops handle expected nodes.

Depends on D154346

I think that HTMLEditor::SetInlinePropertyAsSubAction and
HTMLEditor::RemoveInlinePropertyAsSubAction should not be called multiple
times in one edit action. Therefore, I'd like to make them take multiple
styles once. Then, we could collect all targets before touching the DOM tree
in the future.

Depends on D154348

HTMLEditor::RelativeFontChange() and its helpers are based on
SetInlinePropertiesAsSubAction() and its helpers. Therefore, they may have
similar problem to switch join/split direction. Therefore, this and the
following patches clean them up too.

Depends on D154350

They are renamed to SetFontSizeWithBigOrSmallElement and
SetFontSizeOfFontElementChildren.

Depends on D154351

It should use AutoRangeArray to stop using AutoSelectionRestorer.

Depends on D154352

It's a Gecko specific feature, and it sets background color of parent block
elements of selection ranges. This does similar things to
SetInlinePropertiesAsSubAction, but still refers Selection directly and
uses AutoSelectionRestorer. For consistency between similar methods, this
patch makes it use AutoRangeArray.

Depends on D154353

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/c2c295e5a3ff part 1: Make `HTMLEditor::SplitAncestorStyledInlineElementsAt` and `HTMLEditor::ClearStyleAt` stop touching `Selection` directly r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/52f5e6c77f78 part 2: Make `HTMLEditor::SplitAncestorStyledInlineElementsAtRangeEdges` stop touching `Selection` directly r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/46ae9281c1b2 part 3: Make `HTMLEditor::SetInlinePropertyOnTextNode` stop touching `Selection` directly r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/99211597b4ee part 4: Make `HTMLEditor::RemoveInlinePropertyInternal` use `AutoRangeArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/4f7f2298561e part 5: Make `HTMLEditor::RemoveInlinePropertyAsSubAction` handle correct text node after splitting a text to any direction r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/2001ca2b890e part 6: Make `HTMLEditor::SetInlinePropertyInternal` use `AutoRangeArray` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/81b5e7d21c52 part 7: Make `HTMLEditor::RemoveInlinePropertyAsSubAction` handle multiple styles once r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/e11a81176253 part 8: Make `HTMLEditor::SetInlinePropertyAsSubAction` handle multiple styles once r=m_kato https://hg.mozilla.org/integration/autoland/rev/ea6146e05cc3 part 9: Make `HTMLEditor::RelativeFontChangeOnNode` and `HTMLEditor::RelativeFontChangeHelper` use `FontSize` r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/d08cca9028d6 part 10: Make `HTMLEditor::RelativeFontChangeOnNode` and `HTMLEditor::RelativeFontChangeHelper` stop touching `Selection` directly r=m_kato
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/f7f8eb028923 part 11: Clean up `HTMLEditor::RelativeFontChange` r=m_kato https://hg.mozilla.org/integration/autoland/rev/aa6f8692f8d0 part 12: Make `HTMLEditor::SetCSSBackgroundColorWithTransaction` use similar approach as `SetInlinePropertiesAsSubAction` etc r=m_kato
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: