Closed Bug 1803044 Opened 2 years ago Closed 2 years ago

Reuse existing inline elements to set CSS style even if they have some attributes

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
110 Branch
Tracking Flags:
Tracking Status
firefox110 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(9 files)

Bug 1803044 - part 1: Create a method of `EditorInlineStyle` to check whether an element represents the style r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 2: Improve `HTMLEditUtils::ElementHasAttributeExcept` r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 3: Make `EditorBase::RemoveAttributeWithTransaction` stop creating transaction if the element does not have the removing attribute r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 4: Make `HTMLEditor::SetAttributeOrEquivalent` insert a white-space only when old value is empty r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 5: Make `HTMLEditor::RemoveStyleInside` check whether aElement is editable first r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 6: Make `HTMLEditor::RemoveStyleInside` remove CSS style before touching the DOM tree structure r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 7: Make `HTMLEditor::RemoveStyleInside` use `HTMLEditor::ReplaceContainerWithTransaction` r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 8: Refactor the bottom half of `HTMLEditor::RemoveStyleInside` r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803044 - part 9: Make the style editor use existing inline element in the CSS mode as far as possible r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review

In the following patches, the new variations are required. Additionally,
HTMLEditor should ignore empty class, id and style attributes because
an older API does it.
https://searchfox.org/mozilla-central/rev/2ad13433da20a0749e1e9a10ec0ab49b987c2c8e/editor/libeditor/HTMLStyleEditor.cpp#3398-3402

Depends on D163996

This change makes the following changes smaller. Once we remove CSS style
first, then, it becomes easier to consider whether we should delete the element
since style attribute may be deleted by ChangeStyleTransaction if the
removing style is the last rule in it.

Depends on D164000

Now, we can make there simpler because the part does one of them:

  • Replaces element with new <span>
  • Removes element
  • Removes attribute
  • Does nothing

However, there are duplicated code. We should consider what there does with
lambdas.

Depends on D164002

The other browsers use any inline elements to set CSS property for applying
an inline style. However, Gecko limits it to a <span> which does not have
any attributes. The other browsers' design is better for saving number of
elements and runtime cost of inserting new element (moving all children to the
new element and inserting it to the original position). Therefore, it's nicer
to follow the other browses. Then, we can avoid new WPT failures at aligning
other behaviors to the other browsers.

With doing that, removing style code requires complicated changes because
RemoveStyleInside assumes that one element has one style, but after taking
the compatible behavior, one element can have multiple styles including the
style represented by the element itself.

Note that the new expected failures are caused by bug 1183844. Gecko returns
closest ancestor element's background color for
queryCommandValue("backColor"). Therefore, it returns "transparent" of the
inner <span> element.

Depends on D164003

Summary: Reuse `<span>` element which has some attributes to set CSS style → Reuse existing inline elements to set CSS style even if they have some attributes
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/95ca7cad0c4a
part 1: Create a method of `EditorInlineStyle` to check whether an element represents the style r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3c444b60c784
part 2: Improve `HTMLEditUtils::ElementHasAttributeExcept` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/a067aad1a901
part 3: Make `EditorBase::RemoveAttributeWithTransaction` stop creating transaction if the element does not have the removing attribute r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5a3b614bf90b
part 4: Make `HTMLEditor::SetAttributeOrEquivalent` insert a white-space only when old value is empty r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4ad244ef4253
part 5: Make `HTMLEditor::RemoveStyleInside` check whether aElement is editable first r=m_kato
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37466 for changes under testing/web-platform/tests
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/71f29a225197
part 6: Make `HTMLEditor::RemoveStyleInside` remove CSS style before touching the DOM tree structure r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ceefc7aac79b
part 7: Make `HTMLEditor::RemoveStyleInside` use `HTMLEditor::ReplaceContainerWithTransaction` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/717893e29a82
part 8: Refactor the bottom half of `HTMLEditor::RemoveStyleInside` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/14b38d16c5ed
part 9: Make the style editor use existing inline element in the CSS mode as far as possible r=m_kato
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: