There are 3 bugs.
One is, `PendingStyles::TakeAllPreservedStyles` returns styles in reversed order
of setting styles, this unexpected order causes that `<font>` element is placed
innermost instead of outermost, thus, background color may be applied to shorter
area than text. Unfortunately, Chrome applies the style in fixed order, but
using same order as setting the styles is reasonable for us because we use
same code to apply multiple styles once.
Another is, `AutoInlineStyleSetter` extends the range to apply new style to
parents even if the given point is not in start or end of the first container.
Current code works fine if there is only one text node in an inline element,
but after un-setting a style, text nodes may be keep split. And also when we
apply new style, we create an empty text node for a placeholder of new text,
therefore, the range may be extended from the empty text node to before/after
its previous/next text node. Note that the latter case needs to work with
invisible things more, but this bug needs to be fixed ASAP due to the dead line,
and invisible things not so appear in inline elements. Therefore, it's okay
to leave it.
The other is, the delete handlers do not clear the cached styles (cached when
`HTMLEditor` starts handling it). If it's caused by a `Backspace` key press
in start of a block, `HTMLEditor` caches the style at caret (right paragraph),
then, collapse `Selection` to end of the ex-left block where may have different
style, then, when user types new text there, the cached style is applied into
to the new text **too**. Therefore, if `Selection` was collapsed and handling
a `Backspace` key press, the cached styles should be forgotten.
Depends on D168640
Differential Revision: https://phabricator.services.mozilla.com/D168865
Height of inline elements are considered with current `font-size` and
`line-height`. Therefore, if content in inline elements are taller, the
`background-color` does not fill the bigger content background entirely.
For solving this issue, Chrome handles styles of `<font>` element as
outer-most style. This is reasonable approach, let's follow this.
For solving this issue, we can change the order of `PreservedStyle`s at setting
the preserved styles. Then, `SetInlinePropertiesAsSubAction` is called with
reversed order and apply later style first and applies newer styles to all
content in an element which is previously inserted. Therefore, the `<font>`
element styles should be last elements of `PendingStyles::mPreservingStyles`.
When applying new style, our style editor does not reuse existing `<font>`
element, and this causes writing WPT harder. Therefore, this patch also changes
the applying range of `<font>` style to wrapping existing `<font>` element if
and only if its content is entirely selected.
Unfortunately, this approach cannot get exactly same result as Chrome because we
insert outer-most `<font>` element first, then, try to apply `background-color`,
at this moment, our style editor applies the style to the previously inserted
`<font>` element instead of creating new `<span>` element. This behavior is
required for compatibility in the other cases. Additionally, changing only this
behavior requires a lot of method changes to specify how to handle it. However,
this incompatible behavior may not cause any problems in web apps in the wild.
Therefore, this patch does not solve this incompatible issue. I think that once
we get a bug report caused by this difference, we should redesign how to set
multiple inline styles once.
Differential Revision: https://phabricator.services.mozilla.com/D166617
Height of inline elements are considered with current `font-size` and
`line-height`. Therefore, if content in inline elements are taller, the
`background-color` does not fill the bigger content background entirely.
For solving this issue, Chrome handles styles of `<font>` element as
outer-most style. This is reasonable approach, let's follow this.
For solving this issue, we can change the order of `PreservedStyle`s at setting
the preserved styles. Then, `SetInlinePropertiesAsSubAction` is called with
reversed order and apply later style first and applies newer styles to all
content in an element which is previously inserted. Therefore, the `<font>`
element styles should be last elements of `PendingStyles::mPreservingStyles`.
When applying new style, our style editor does not reuse existing `<font>`
element, and this causes writing WPT harder. Therefore, this patch also changes
the applying range of `<font>` style to wrapping existing `<font>` element if
and only if its content is entirely selected.
Unfortunately, this approach cannot get exactly same result as Chrome because we
insert outer-most `<font>` element first, then, try to apply `background-color`,
at this moment, our style editor applies the style to the previously inserted
`<font>` element instead of creating new `<span>` element. This behavior is
required for compatibility in the other cases. Additionally, changing only this
behavior requires a lot of method changes to specify how to handle it. However,
this incompatible behavior may not cause any problems in web apps in the wild.
Therefore, this patch does not solve this incompatible issue. I think that once
we get a bug report caused by this difference, we should redesign how to set
multiple inline styles once.
Differential Revision: https://phabricator.services.mozilla.com/D166617
The problem in Yahoo mail is, when you apply new font size to:
```
<div><font size="7">{}<br></font></div>
```
then, when you type text, you'll get:
```
<div><font size="7"><font size="4">abc<br></font></font></div>
```
because `HTMLEditor::CreateStyleForInsertText` does not check ancestors to
apply new style to empty text node which is created by the method for
placeholder. Of course, the other browsers update the existing
`<font size="7">` instead and we should follow it.
However, there are 3 problems:
1. Our editor may adjust insertion point to nearest editable text node if caret
in empty inline elements.
2. Our editor supports increase/decrease font size which have been removed from
`Document.execCommand`, but Thunderbird keeps using it, and that's implemented
with an independent method.
3. Our style editor code does not support applying style to collapsed selection.
Therefore, this patch does:
1. keep create empty text node to apply new style at specific point.
2. give up to use new path if there is preserved font size increment/decrement.
3. separate `HTMLEditor::SetInlinePropertiesAsSubAction` and add new path for
applying style to collapsed range.
Then, `HTMLEditor::CreateStyleForInsertText` can use
`HTMLEditor::SetInlinePropertiesAsSubAction` which supports handling new styles
with existing ancestors.
Differential Revision: https://phabricator.services.mozilla.com/D166416
Height of inline elements are considered with current `font-size` and
`line-height`. Therefore, if content in inline elements are taller, the
`background-color` does not fill the bigger content background entirely.
For solving this issue, Chrome handles styles of `<font>` element as
outer-most style. This is reasonable approach, let's follow this.
For solving this issue, we can change the order of `PreservedStyle`s at setting
the preserved styles. Then, `SetInlinePropertiesAsSubAction` is called with
reversed order and apply later style first and applies newer styles to all
content in an element which is previously inserted. Therefore, the `<font>`
element styles should be last elements of `PendingStyles::mPreservingStyles`.
When applying new style, our style editor does not reuse existing `<font>`
element, and this causes writing WPT harder. Therefore, this patch also changes
the applying range of `<font>` style to wrapping existing `<font>` element if
and only if its content is entirely selected.
Unfortunately, this approach cannot get exactly same result as Chrome because we
insert outer-most `<font>` element first, then, try to apply `background-color`,
at this moment, our style editor applies the style to the previously inserted
`<font>` element instead of creating new `<span>` element. This behavior is
required for compatibility in the other cases. Additionally, changing only this
behavior requires a lot of method changes to specify how to handle it. However,
this incompatible behavior may not cause any problems in web apps in the wild.
Therefore, this patch does not solve this incompatible issue. I think that once
we get a bug report caused by this difference, we should redesign how to set
multiple inline styles once.
Depends on D166416
Differential Revision: https://phabricator.services.mozilla.com/D166617
The problem in Yahoo mail is, when you apply new font size to:
```
<div><font size="7">{}<br></font></div>
```
then, when you type text, you'll get:
```
<div><font size="7"><font size="4">abc<br></font></font></div>
```
because `HTMLEditor::CreateStyleForInsertText` does not check ancestors to
apply new style to empty text node which is created by the method for
placeholder. Of course, the other browsers update the existing
`<font size="7">` instead and we should follow it.
However, there are 3 problems:
1. Our editor may adjust insertion point to nearest editable text node if caret
in empty inline elements.
2. Our editor supports increase/decrease font size which have been removed from
`Document.execCommand`, but Thunderbird keeps using it, and that's implemented
with an independent method.
3. Our style editor code does not support applying style to collapsed selection.
Therefore, this patch does:
1. keep create empty text node to apply new style at specific point.
2. give up to use new path if there is preserved font size increment/decrement.
3. separate `HTMLEditor::SetInlinePropertiesAsSubAction` and add new path for
applying style to collapsed range.
Then, `HTMLEditor::CreateStyleForInsertText` can use
`HTMLEditor::SetInlinePropertiesAsSubAction` which supports handling new styles
with existing ancestors.
Differential Revision: https://phabricator.services.mozilla.com/D166416
As of the prior patch, these are no longer needed. I removed
these with a script, then ran clang-format on the files, then
manually reverted a few unrelated changed from the formatter.
Differential Revision: https://phabricator.services.mozilla.com/D164829
nsISupports.h includes nsISupportsBase.h, so it should be equivalent.
In the next patch, I'm changing things so that nsISupports is defined in
nsISupports.h instead of nsISupportsBase.h, and deleting the latter, so
this change will be needed anyways. I'm guessing people were using IWYU
or something like that.
Differential Revision: https://phabricator.services.mozilla.com/D159169
They are used in public inline methods of `HTMLEditor`. Therefore, they should
be defined in an exposed header file without other internal use things for the
editor module. Then, we can stop exposing `HTMLEditHelpers.h`.
Depends on D158632
Differential Revision: https://phabricator.services.mozilla.com/D158633
Additionally,
* `PropItem` -> `PendingStyle`
* `StyleCache` -> `PendingStyleCache`
* `AutoStyleCacheArray` -> `AutoPendingStyleCacheArray`
And finally, `PendingStyle` (formally `PropItem`) is changed to `class` and
its members are encapsuled.
Differential Revision: https://phabricator.services.mozilla.com/D155318
serge-sans-paille