Editor should stop using non-standardized attributes to put `<br>` elements for padding of empty lines
Categories
(Core :: DOM: Editor, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(5 files)
Editor creates <br mozbogusnode="true"> for empty editor and <br type="_moz"> for empty last line of <textarea> and blocks in HTML editor (without such hacky <br> elements, caret position is computed with 0-height line). However, those attributes can be modified by web apps and might conflict with future spec. Additionally, the runtime cost of setting/removing/checking the attributes may appear in profiles.
I believe that editor should treat HTMLBRElement directly and HTMLBRElement should have specific flags which mean each of the attributes.
Then, we can save the runtime cost, especially mutation observers (including IMEContentObserver).
|
Assignee | |
Comment 1•5 years ago
|
||
Editor creates a <br> element when it's root element is empty.
Then, it's stored by TextEditRules::mBogusNode and used for checking
whether the editor is empty quickly. However, this <br> element has
mozeditorbogusnode attribute whose value is true. However, adding or
removing the attribute is not cheap and web apps can refer such illegal
attribute.
Therefore, this patch makes HTMLBRElement take a specific flag whether
it's a bogus node or not. However, this means that this hacky thing will be
exposed outside editor module. For making what is the bogus node clearer,
this patch calls the such <br> elements as "padding <br> element for
empty editor". So, this patch also includes a lot of renaming methods and
variables, and modifying related comments.
|
|
Assignee | |
Comment 2•5 years ago
|
||
Editor creates a <br> element to end of a block if last line
of the block is empty because caret should be placed as there is an empty
line. Such special <br> element has type attribute whose value is "_moz".
However, adding/removing the attribute is expensive and such hacky attribute
shouldn't be referred nor changed by web apps.
Therefore, this patch makes HTMLBRElement take another specific flag whether
it's a special node for empty last line. For making the meaning clearer,
this patch calls the such <br> elements as "padding <br> element for
empty last line" insead of "moz-br". So, this patch also includes a lot of
renaming methods and variables, and modifying related comments.
Note that with this change, IMEContentObserver counts the padding <br>
element in <textarea> because it's inserted before setting the new flag
and setting the flag does not cause DOM tree mutation. This issue will be
fixed by the following patches.
|
|
Assignee | |
Comment 3•5 years ago
|
||
TextEditRules::CreateBR() is used only by
HTMLEditRules::InsertBRIfNeededInternal() and it just calls
TextEditor::InsertBrElementWithTransaction(). Therefore, we can get rid of
it. Then, CreateBRInternal() can be renamed to
CreatePaddingBRElementForEmptyLastLine() since it's shared only by
CreateBR() and CreatePaddingBRElementForEmptyLastLine().
|
|
Assignee | |
Comment 4•5 years ago
|
||
Stopping using attribute for "moz-br", IMEContentObserver cannot know when
new <br> element is changed to padding element for empty last line.
Therefore, editor needs to insert padding <br> element after setting the
flag properly. Then, IMEContentObserver does not need to recompute the
length of <br> element (if it's for padding, it computes the length as 0).
Unfortunately, TextEditor::InsertBrElementWithTransaction() is used in too
many places and it already has optional argument. Therefore, it's difficult
to change it. However, we should share the preparation before creating <br>
element in it with new method. Therefore, this patch creates
EditorBase::PrepareToInsertBRElement() to share the preparation point (almost
just moved from the method). Then, new method is created as
EditorBase::InsertPaddingBRElementForEmptyLastLineWithTransaction() because
it's used both in TextEditor and HTMLEditor. Finally, TextEditor won't
insert <br> element with InsertBrElementWithTransaction(). Therefore, it's
moved to HTMLEditor with renaming to InsertBRElementWithTransaction().
|
|
Assignee | |
Comment 5•5 years ago
|
||
IMEContentObserver treats <br> elements as linefeed, but ignores padding
<br> elements. Padding <br> element for last empty line was inserted as
normal <br> element and then, its attribute was set by editor. Therefore,
IMEContentObserver needed to observe attribute changes. However, editor
stops using attribute to mark as padding and stops inserting as normal <br>
element first (i.e., inserting <br> after setting its flag). Therefore,
IMEContentObserver does not need to observe attribute changes anymore.
|
||
Comment 7•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/53fd693e44a7
https://hg.mozilla.org/mozilla-central/rev/d3d34552e75d
https://hg.mozilla.org/mozilla-central/rev/b9ffd7bff146
https://hg.mozilla.org/mozilla-central/rev/5a67c3b41886
https://hg.mozilla.org/mozilla-central/rev/483c129b10e3
|
|
Assignee | |
Updated•5 years ago
|
Description
•