Finally, `Document.execCommand()` still does not work fine if selection
starts from very start of block and/or end at very end of block because
`PromoteInlineRange()` extends selection range to contain the
containers, then, `SubtreeContentIterator` won't list up text nodes.
In this case, `RemoveInlinePropertyInternal()` expects that
`RemoveStyleInside()` removes text node style with creating
`<span>` elements. However, `RemoveStyleInsilde()` only handles
`Element`s and it handles elements from most-descendants.
Therefore, it cannot distinguish whether text node style comes
from removing inline elements or parent block.
This patch makes `RemoveInlinePropertyInternal()` collect
descendant text nodes in the range after handling all nodes in
the range except descendant text nodes, then, check the
final style of descendant text nodes, finally, remove the style
if coming from parent block.
Differential Revision: https://phabricator.services.mozilla.com/D47865
--HG--
extra : moz-landing-system : lando
If selection range is not in **one** text node, `RemoveInlinePropertyInternal()`
collects target nodes with `SubtreeContentIterator`. It only collects topmost
nodes which are **entirely** contained in the range (it's enough because their
descendants will be handled by `RemoveStyleInside()` recursively).
The reasons why it uses `SubtreeContentIterator` rather than
`PreContentIterator` must be:
1. Performance reason.
2. Assuming there are no multiple text nodes.
3. Not expects that user removes text node styles come from parent block.
The reason 2 is wrong because when removing a style, all browsers don't
join text nodes which was in removing element with adjacent text nodes.
(I.e., we cannot change this behavior for compatibility.)
The reason 3 is of course wrong we're struggling with this scenario.
Therefore, `RemoveInlinePropertyInternal()` needs to collect partially
selected text nodes by itself (if there are). Then, we can merge the
single text node selected case with the `for` loop.
Differential Revision: https://phabricator.services.mozilla.com/D47864
--HG--
extra : moz-landing-system : lando
For compatibility with Chrome, when removing inline style at block parent,
we should reset the style with creating `<span>` element whose `style`
attribute removes the style. We do this only in CSS mode, but we should do
it in HTML mode too.
This patch also makes `FontFaceStateCommand::SetState()` ignore `tt` value
if its root caller is `Document::ExecCommand()`. It was implemented for
composer to handle XUL command in bug 115922. Therefore, we should not do
this special handling on the web. If it were possible to separate this
change to another bug, it'd be nicer. But without this change, we'll have
a lot of regressions of `Document.execCommand("fontname")`. Therefore,
this is also fixed in this patch.
Note that this removes first `.ini` file selection because
the tests cannot be run without test number range parameter.
So, the sections are not used anymore.
Differential Revision: https://phabricator.services.mozilla.com/D47862
--HG--
extra : moz-landing-system : lando
For compatibility with Chrome, when removing inline style at block parent,
we should reset the style with creating `<span>` element whose `style`
attribute removes the style. We do this only in CSS mode, but we should do
it in HTML mode too.
This patch also makes `FontFaceStateCommand::SetState()` ignore `tt` value
if its root caller is `Document::ExecCommand()`. It was implemented for
composer to handle XUL command in bug 115922. Therefore, we should not do
this special handling on the web. If it were possible to separate this
change to another bug, it'd be nicer. But without this change, we'll have
a lot of regressions of `Document.execCommand("fontname")`. Therefore,
this is also fixed in this patch.
Note that this removes first `.ini` file selection because
the tests cannot be run without test number range parameter.
So, the sections are not used anymore.
Differential Revision: https://phabricator.services.mozilla.com/D47862
--HG--
extra : moz-landing-system : lando
Surprisingly, its `aChildOnly` is never set to `true` and if it were set to
`true`, it does unnecessary recursive calls. Therefore, we can make it
simpler.
Differential Revision: https://phabricator.services.mozilla.com/D47860
--HG--
extra : moz-landing-system : lando
This is related to bug 1530649. When using <span> element with contentedtiable,
we won't insert <br> element at last. When Korean IME on macOS commits
composition by space key, composition string has space.
Gekco removes U+0020 space when it is last character into editing host. To keep
whitespace, we have to replace with NBSP when it is last.
Differential Revision: https://phabricator.services.mozilla.com/D48146
--HG--
extra : moz-landing-system : lando
Both method take a DOM point with `nsCOMPtr<nsINode>*` and `int32_t*`.
This makes each caller complicated. Instead, we should use stack only class
to return both `EditorDOMPoint` and `nsresult`. I name it `EditResult`.
Additionally, this fixes a bug of `HTMLeditor::SplitStyleAboveRange()`. That
is not tracking new selection start point while it splits elements at end
of given range. This is detected by the debug assertion in
`ToRawRangeBoundary()` (i.e., this fix is required to pass some tests).
Differential Revision: https://phabricator.services.mozilla.com/D47858
--HG--
extra : moz-landing-system : lando
Most of these tests have been disabled for a long time; they run well
in the current test environment.
Differential Revision: https://phabricator.services.mozilla.com/D47616
--HG--
extra : moz-landing-system : lando
Instead, rely on the proper initialization of `mWrapColumn` via
`nsPlainTextSerializer::Init`.
This will allow to move `mWrapColumn` to `nsPlainTextSerializer::Settings::mWrapColumn`,
simplifying reasoning about it.
Differential Revision: https://phabricator.services.mozilla.com/D46129
--HG--
extra : moz-landing-system : lando
Currently, we can use chrome process's shortcut key with
`EventUtils.synthesizeKey()` with enabling `"test.events.async.enabled"` pref.
So, we should reconvert it to a mochitest for making it more stable.
Oddly, when I try to run this test as test-verify on macOS, it permanently
fails rendering resizer of `<textarea>` elements immediately after creation.
Therefore, this patch disables this test in test-verify on macOS.
Differential Revision: https://phabricator.services.mozilla.com/D46578
--HG--
rename : editor/libeditor/tests/browser_bug629172.js => editor/libeditor/tests/test_bug629172.html
extra : moz-landing-system : lando
Only for solving the order of `AutoEditInitRulesTrigger` destruction when
`HTMLEditor::Init()` calls `TextEditor::Init()`, `TextEditor` has unnecessary
counter and the initialization code is made harder to understand.
Therefore, this patch makes `TextEditor::Init()` and `HTMLEditor::Init()`
directly call `InitEditorContentAndSelection()` at appropriate time.
Additionally, `HTMLEditor::Init()` can call `EditorBase::Init()` instead of
`TextEditor::Init()` since `TextEditor::Init()` does nothing for `HTMLEditor`
actually.
Differential Revision: https://phabricator.services.mozilla.com/D45793
--HG--
extra : moz-landing-system : lando
Now, we can get rid of `TextEditRules` and `HTMLEditRules` completely.
And also this patch renames their cpp files to `TextEditSubActionHandler`
and `HTMLEditSubActionHandler`.
`TextEditor::Init()` and `HTMLEditor::Init()` are still complicated due to
`AutoEditInitRulesTrigger`. The following patch will remove it.
Differential Revision: https://phabricator.services.mozilla.com/D45792
--HG--
rename : editor/libeditor/HTMLEditRules.cpp => editor/libeditor/HTMLEditSubActionHandler.cpp
rename : editor/libeditor/TextEditRules.cpp => editor/libeditor/TextEditSubActionHandler.cpp
extra : moz-landing-system : lando
It takes a lot of `bool` out arguments. Therefore, we should make it a
stack only class and caller should retrieve only necessary information.
Differential Revision: https://phabricator.services.mozilla.com/D45787
--HG--
extra : moz-landing-system : lando
And also this patch make each `AutoEditSubActionNotifier` creator check
the result of `OnStartToHandleTopLevelEditSubAction()` at least for
`NS_ERROR_EDITOR_DESTROYED`.
We need to take care of its destructor's result later, though.
Differential Revision: https://phabricator.services.mozilla.com/D45786
--HG--
extra : moz-landing-system : lando
This is regression by bug 1530649.
After landing bug 1530649, we try to scan end point of replacement text. But
in this bug's situation, afterRun becomes same as current ws run by landing
bug 1530649. To get white space type of next of replacement end, we have to
scan around end point again.
Differential Revision: https://phabricator.services.mozilla.com/D45947
--HG--
extra : moz-landing-system : lando
`TextEditRules::DocumentIsEmpty()` is a wrapper of `TextEditor::IsEmpty()` so
that we can get rid of it simply.
`HTMLEditRules::DocumentIsEmpty()` needs to change. It's oddly checks only
`EditorBase::mPaddingBRElementForEmptyEditor` is `nullptr` or not. However,
the editor may be completely empty. And the result may be different from
`TextEditor::IsEmpty()` which is not overridden by `HTMLEditor`. For partially
solving this issue, this patch makes `HTMLEditor` overrides `IsEmpty()` and
optimizes `TextEditor::IsEmpty()`.
With this change, the caller of `HTMLEditRules::DocumentIsEmpty()` may behave
differently in the only caller, `HTMLEditor::SelectEntireDocument()`. And
unfortunately, its root called from `SelectAllCommand::DoCommand()`. However,
it does just collapse `Selection` into the root element (`<body>` or
`Document.documentElement`) if it returns `true`. Therefore, this change
must be safe since anyway `SelectionRefPtr()->SelectAllChildren()` with
the root element is exactly same as `SelectionRefPtr()->Collapse()` with
the empty root element if it's truly empty.
Differential Revision: https://phabricator.services.mozilla.com/D45784
--HG--
extra : moz-landing-system : lando
The main purpose of them is modifying
`TopLevelEditSubActionData::mChangedRange`. Therefore, they should be in the
struct.
Differential Revision: https://phabricator.services.mozilla.com/D45783
--HG--
extra : moz-landing-system : lando
It requires different preparation in `TextEditor` and `HTMLEditor` but it's
not split. Therefore, we should make it virtual and override it to use
different preparation code. Fortunately, its code is enough simple to
duplicate.
Additionally, this removes unnecessary code from `TextEditRules` and
`HTMLEditRules` including `WillDoAction()` and `DidDoAction()`!
Differential Revision: https://phabricator.services.mozilla.com/D45495
--HG--
extra : moz-landing-system : lando
Unfortunately, `EditSubAction::eInsertElement` is used by 4 methods and one
of them is too big method. But we should move what old `WillInsert()` does
into them for stop making anybody use `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D45494
--HG--
extra : moz-landing-system : lando
It does 4 different things so that it looks like a black-box from the
callers.
First, only `HTMLEditRules::WillDoAction()` refers `aCancel` out argument.
Therefore, it should check whether it's cancelled or not directly.
Next, `EnsureNoPaddingBRElementForEmptyEditor()` should be called by each
caller directly.
Then, the renaming part can be split to 2 methods. One is adjusting
caret position and the other preparing inline style for new content.
Unfortunately, this patch makes each caller messy. I think that for the
3rd job (i.e., adjusting caret position), each caller should retrieve the
adjusted caret position and use it directly instead of handling with
`Selection` in the future.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
It does 4 different things so that it looks like a black-box from the
callers.
First, only `HTMLEditRules::WillDoAction()` refers `aCancel` out argument.
Therefore, it should check whether it's cancelled or not directly.
Next, `EnsureNoPaddingBRElementForEmptyEditor()` should be called by each
caller directly.
Then, the renaming part can be split to 2 methods. One is adjusting
caret position and the other preparing inline style for new content.
Unfortunately, this patch makes each caller messy. I think that for the
3rd job (i.e., adjusting caret position), each caller should retrieve the
adjusted caret position and use it directly instead of handling with
`Selection` in the future.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
