Commit Graph

381 Commits

Author SHA1 Message Date
Masayuki Nakano
6d224d7259 Bug 1549270 - part 2: Mark EditorBase::RemoveAttributeWithTransaction() as MOZ_CAN_RUN_SCRIPT r=m_kato
Differential Revision: https://phabricator.services.mozilla.com/D30044

--HG--
extra : moz-landing-system : lando
2019-05-08 05:09:56 +00:00
Masayuki Nakano
0925cb9a70 Bug 1549155 - Mark EditorBase::DeleteNodeWithTransaction() as MOZ_CAN_RUN_SCRIPT r=m_kato
Differential Revision: https://phabricator.services.mozilla.com/D30038

--HG--
extra : moz-landing-system : lando
2019-05-07 22:27:29 +00:00
Masayuki Nakano
a337925c38 Bug 1549148 - Mark EditorBase::InsertTextIntoTextNodeWithTransaction() as MOZ_CAN_RUN_SCRIPT r=m_kato
Differential Revision: https://phabricator.services.mozilla.com/D30036

--HG--
extra : moz-landing-system : lando
2019-05-07 10:58:08 +00:00
Masayuki Nakano
9405d61cb8 Bug 1546577 - Make overloads of CanCut(), CanCopy(), CanDelete() and CanPaste() which return bool instead of nsresult r=m_kato
`CanCut()`, `CanCopy()` and `CanPaste()` return error only when the editor has
already been destroyed or not been initialized yet, or when failed to access
clipboard when the document is not HTML/XHTML.

`CanDelete()` returns error only when the editor has already been destroyed or
not been initialized yet.

So, these error result won't be exposed to the web in most cases and such
exception shouldn't stop any content script because Chrome basically does not
throw exception in such situation as far as I know.

Therefore, there should be overloads of them to return `bool` result directly
for making their callers simpler.

Differential Revision: https://phabricator.services.mozilla.com/D28608

--HG--
extra : moz-landing-system : lando
2019-04-25 07:14:39 +00:00
Masayuki Nakano
414509fe00 Bug 1543315 - part 9: Mark nsIPresShell::FlushPendingNotifications() as MOZ_CAN_RUN_SCRIPT r=smaug
So, this patch makes all caller of it safe including its arguments unless
they come from other methods.

Differential Revision: https://phabricator.services.mozilla.com/D27225

--HG--
extra : moz-landing-system : lando
2019-04-13 12:43:57 +00:00
Masayuki Nakano
a50a0700d4 Bug 1530649 - Improve composition string handling which ends with whitespaces r=m_kato
If insertion string ends with ASCII whitespace and there is no following
content in the block, `HTMLEditRules::AdjustWhitespaces()` needs to insert
`<br>` element.  It's called only by `HTMLEditRules::AfterEditInner()` and
that does only simple things with `WSRunObject`.  Therefore, this moves the
code into `AfterEditInner()`.

For making it adjust the whitespaces, `HTMLEditRules::WillInsertText()` needs
to notify `AfterEditInner()` of dirty range with `mDocChangeRange`.  Therefore,
this patch makes it set `mDocChangeRange` manually after inserting composition
string.

On the other hand, there is another bug.  `WSRunObject` was designed to treat
only inserting text for `WSRunObject::InsertText()`.  I.e., not designed to
treat replacing existing composition string with new string.  Therefore,
`WSRunObject::InsertText()` adjusts whitespaces only around start of
composition string.  Therefore, if composition string ends with an ASCII
whitespace, it's not replaced with NBSP and that causes:
- failing `WSRunObject::AdjustWhitespaces()` inserts `<br>` element at
  `AfterEditInner()` of committing composition.
- then, next composition's first `WSRunObject::InsertText()` removes the
  last whitespace due to not followed by `<br>` nor any other content.
Therefore, this patch makes `WSRunObject` takes 2 DOM points to be able to
treat replaced range.

In strictly speaking, the latter change require more changes and tests for
supporting replacement with any other methods.  However, it's risky and out
of scope of this bug.

Differential Revision: https://phabricator.services.mozilla.com/D26423

--HG--
extra : moz-landing-system : lando
2019-04-09 05:28:38 +00:00
Masayuki Nakano
3e64f2c30d Bug 1539356 - Mark EditorBase::InsertNodeTransaction() as MOZ_CAN_RUN_SCRIPT r=m_kato
This patch marks `EditorBase::InsertNodeTransaction()` **and** its callers as `MOZ_CAN_RUN_SCRIPT`.

Unfortunately, this patch tells us that some `GetSomething()` methods may destroy the editor since `HTMLEditRules::GetNodesForOperation()`, `HTMLEditRules::GetNodesFromPoint()` and `HTMLEditRules::GetNodesFromSelection()` may change the DOM tree.  Additionally, initialization methods may destroy the editor since it may insert a bogus `<br>` node.

Note that this patch also removes some unused methods. I.e., they are not result of some cleaning up the code. This patch just avoids marking unused methods as `MOZ_CAN_RUN_SCRIPT`.

Differential Revision: https://phabricator.services.mozilla.com/D25027

--HG--
extra : moz-landing-system : lando
2019-03-30 11:55:29 +00:00
Oana Pop Rus
43fadb6745 Backed out changeset 447c87c2d139 (bug 1539356) on request of Jorg K. a=backout 2019-03-30 00:42:32 +02:00
Masayuki Nakano
81b30d7143 Bug 1539356 - Mark EditorBase::InsertNodeTransaction() as MOZ_CAN_RUN_SCRIPT r=m_kato
This patch marks `EditorBase::InsertNodeTransaction()` **and** its callers as `MOZ_CAN_RUN_SCRIPT`.

Unfortunately, this patch tells us that some `GetSomething()` methods may destroy the editor since `HTMLEditRules::GetNodesForOperation()`, `HTMLEditRules::GetNodesFromPoint()` and `HTMLEditRules::GetNodesFromSelection()` may change the DOM tree.  Additionally, initialization methods may destroy the editor since it may insert a bogus `<br>` node.

Note that this patch also removes some unused methods. I.e., they are not result of some cleaning up the code. This patch just avoids marking unused methods as `MOZ_CAN_RUN_SCRIPT`.

Differential Revision: https://phabricator.services.mozilla.com/D25027

--HG--
extra : moz-landing-system : lando
2019-03-29 10:55:31 +00:00
Masayuki Nakano
534fd23ca4 Bug 1533293 - part 2: Rewrite EditorBase::SelectEntireDocument() and its overrides r=m_kato
`EditorBase::SelectEntierDocument()` uses `Selection::Extend()` but it's too
slow.  It should use `Selection::SetStartAndEndInLimiter()` instead.

Additionally, `TextEditor::SelectEntierDocument()` shrink the result of
`EditorBase::SelectEntierDocument()` with `Selection::Extend()` if there is
a `moz-<br>` element.  So, `TextEditor::SelectEntinerDocument()` should set
its expected selection with a call for saving the runtime cost.

Then, we don't need to make `EditorBase::SelectEntierDocument()` as non-pure
virtual method.  So, this patch makes each its callers call
`Selection->SelectAllChildren()` directly.

Differential Revision: https://phabricator.services.mozilla.com/D23461

--HG--
extra : moz-landing-system : lando
2019-03-26 10:06:43 +00:00
Noemi Erli
165f0d8c1c Backed out 3 changesets (bug 1533293) for causing Bug 1536595 a=backout
Backed out changeset d011dfe83683 (bug 1533293)
Backed out changeset e536f6e123d8 (bug 1533293)
Backed out changeset 19cff61f4fed (bug 1533293)
2019-03-20 13:29:17 +02:00
Masayuki Nakano
448571fd81 Bug 1533293 - part 2: Rewrite EditorBase::SelectEntireDocument() and its overrides r=m_kato
`EditorBase::SelectEntierDocument()` uses `Selection::Extend()` but it's too
slow.  It should use `Selection::SetStartAndEndInLimiter()` instead.

Additionally, `TextEditor::SelectEntierDocument()` shrink the result of
`EditorBase::SelectEntierDocument()` with `Selection::Extend()` if there is
a `moz-<br>` element.  So, `TextEditor::SelectEntinerDocument()` should set
its expected selection with a call for saving the runtime cost.

Then, we don't need to make `EditorBase::SelectEntierDocument()` as non-pure
virtual method.  So, this patch makes each its callers call
`Selection->SelectAllChildren()` directly.

Differential Revision: https://phabricator.services.mozilla.com/D23461

--HG--
extra : moz-landing-system : lando
2019-03-18 01:51:53 +00:00
Masayuki Nakano
39daaa7db8 Bug 1534561 - Make editor use PresShell directly rather than nsIPresShell r=m_kato
`PresShell.h` is exposed as `mozilla/PresShell.h` and `PresShell` is the only
concrete class of `nsIPresShell`.  Therefore, we have no reason to access
`PresShell` via `nsIPresShell`.

Differential Revision: https://phabricator.services.mozilla.com/D23277

--HG--
extra : moz-landing-system : lando
2019-03-15 05:01:10 +00:00
Boris Zbarsky
9a4ba73134 Bug 1534370 part 3. Mark InsertFromTransferable as MOZ_CAN_RUN_SCRIPT. r=masayuki
Differential Revision: https://phabricator.services.mozilla.com/D23042

--HG--
extra : moz-landing-system : lando
2019-03-12 01:55:03 +00:00
Masayuki Nakano
9609df9a73 Bug 1525481 - part 3: Make editor not expose internal errors to the web r=m_kato
As far as I've tested, Chrome does not throw exception even when editor is
destroyed or editor content is modified unexpectedly.  So, we should return
`NS_OK` from most public methods of editor when internal methods return
`NS_ERROR_EDITOR_DESTROYED` or `NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE`.

Differential Revision: https://phabricator.services.mozilla.com/D20811

--HG--
extra : moz-landing-system : lando
2019-02-25 09:07:54 +00:00
Masayuki Nakano
0b16be82cb Bug 1525481 - part 2: Make EditorBase::DoSplitNode() return error if split nodes are moved/removed unexpectedly r=m_kato
We should stop handling splitting nodes if mutation event listeners move or
remove the split nodes unexpectedly because the post processors may not be
able to keep handling the nodes.  For example, if a node is moved to outside
of editing host, we shouldn't touch it anymore due to non-editable.

This patch makes `EditorBase::DoSplitNode()` return new error for making
any parent callers stop their job, but note that the following patch makes
any public methods expose the new error as exception for compatibility with
Chrome.

Differential Revision: https://phabricator.services.mozilla.com/D20810

--HG--
extra : moz-landing-system : lando
2019-02-25 08:29:38 +00:00
Masayuki Nakano
f8a317eb9a Bug 998941 - part 2-3: Create new constructors of DataTransfer to set only plain text or nsITransferable r=smaug
DataTransfer should have more 2 constructors.  One takes |const nsAString&| and
sets its data to the string.  The other takes |nsITransferable*| and stores only
its data with DataTransferItem.

If given data is external resource like the case of `HTMLEditor::PasteTransferable()`,
we should copy its data to each DataTransferItem because nsITransferable is not
cycle-collectable, but DataTransfer may be grabbed by JS.  Unfortunately, adding
new path to initialize DataTransfer with nsITransferable instance is too risky
because DataTransfer and DataTransferItem work together to initialize each of
them if DataTransfer is in external mode. Therefore, this patch makes the
new constructor temporarily sets it to in external mode, then, cache usable types
first, then, call `FillAllExternalData()` to  make each DataTransferItem initializes
its data by itself, finally, make the constructor set it to internal mode and release
nsITransferable instance.  This is ugly implementation but the most reasonable
way for now because:

- We don't need to change DataTransfer nor DataTransferItem a lot in this bug.
- We don't need to duplicate any code like a loop in `CacheExternalData()`.

In another bug, we should redesign DataTransfer and DataTransferItem to
make DataTransferable easier to be added new constructors.

Differential Revision: https://phabricator.services.mozilla.com/D19298

--HG--
extra : moz-landing-system : lando
2019-02-19 07:13:20 +00:00
Masayuki Nakano
474225038e Bug 998941 - part 2-2: Make HTMLEditor set InputEvent.dataTransfer when InputEvent.inputType is "insertFromPaste", "insertFromDrop" or "insertReplacementText" r=smaug,m_kato
InputEvent.dataTransfer should be set to non-null when InputEvent.inputType
is "insertFromPaste", "insertFromDrop" or "insertReplacementText" and
editor is an HTMLEditor instance:
https://rawgit.com/w3c/input-events/v1/index.html#dfn-data
https://w3c.github.io/input-events/#dfn-data

("insertTranspose" and "insertFromYank" are not currently supported on Gecko.)

This patch makes nsContentUtils::DispatchInputEvent() take dataTransfer value
and EditorBase set it via AutoEditActionDataSetter like data value.

However, we need to create other constructors of DataTransfer to create its
read-only instances initialized with nsITransferable or nsAString.  This will
be implemented by the following patch.

Differential Revision: https://phabricator.services.mozilla.com/D19297

--HG--
extra : moz-landing-system : lando
2019-02-19 06:33:42 +00:00
Masayuki Nakano
551de121c1 Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to serialized color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio
Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. https://github.com/w3c/input-events/issues/94#issuecomment-461061517

Differential Revision: https://phabricator.services.mozilla.com/D19295

--HG--
extra : moz-landing-system : lando
2019-02-19 06:31:28 +00:00
Masayuki Nakano
122ab2691a Bug 998941 - part 1-4: Make editor set InputEvent.data to "ltr" or "rtl" when InputEvent.inputType is "formatSetBlockTextDirection" r=smaug,m_kato
When InputEvent.inputType is "formatSetBlockTextDirection" or
"formatSetInlineTextDirection", InputEvent.data value should be one of
"ltr", "rtl", "auto" or "null".
https://rawgit.com/w3c/input-events/v1/index.html#dfn-data
https://w3c.github.io/input-events/#dfn-data

We only supports "ltr" and "rtl" when user switches the direction with
Accel + Shift + X.  Therefore this patch makes EditorBase set the data
to "ltr" or "rtl".

Oddly, with synthesizing the shortcut keys, the command is not executed
properly in the automated test.  Therefore, this patch dispatches the
command directly.

Differential Revision: https://phabricator.services.mozilla.com/D19288

--HG--
extra : moz-landing-system : lando
2019-02-19 06:29:38 +00:00
Masayuki Nakano
a3484e40c0 Bug 998941 - part 1-2: Make editor set InputEvent.data to inserting text when it sets InputEvent.inputType to "insertText" or "insertCompositionText" r=smaug,m_kato
This patch makes nsContentUtils::DispatchInputEvent() support to set
InputEvent.data.  Whether the its value should be null or DOMString depends
on InputEvent.inputType value.

- https://rawgit.com/w3c/input-events/v1/index.html#overview
- https://rawgit.com/w3c/input-events/v1/index.html#dfn-data
- https://w3c.github.io/input-events/#overview
- https://w3c.github.io/input-events/#dfn-data

According to the draft specs, InputEvent.data should be always inserting text
when inputType is "insertText" or "insertCompositionText" (or
"insertFromCompoition" if Level 2 support is enabled).

Differential Revision: https://phabricator.services.mozilla.com/D19286

--HG--
extra : moz-landing-system : lando
2019-02-19 06:28:19 +00:00
Makoto Kato
c7936919a5 Bug 1518002 - Update selection cache when initializing editor. r=masayuki
GitLab's comment calls scrollTop on input event handler. The scollTop may cause
reflow.  When causing reflow, editor is destroyed and initialized again. Then
nsTextEditorState will set current value to editor.  But this is failure.

By bug 1465702, selection is cached in edit action. When initializing editor,
selection controller is updated, so selection into cache becomes invalid. It
means that all selection methods will return error since document is different.

So we should update selection cache when initializing editor.

Also, I cannot create test case for this situation since we have to cause reflow
in input and/or composition event. Do you have any idea?

Differential Revision: https://phabricator.services.mozilla.com/D16944

--HG--
extra : moz-landing-system : lando
2019-01-18 10:00:23 +00:00
Masayuki Nakano
ed2040ac16 Bug 1447239 - Implement InputEvent.inputType r=smaug
This patch implements InputType.inputType which is declared by Input Events.
The attribute has already been implemented by Chrome and Safari.  Chrome
implements Input Events Level 1, but Safari implements Input Events Level 2.
 Difference between them is only whether it supports "insertFromComposition",
"deleteByComposition" and "deleteCompositionText".  This patch makes the
level switchable with pref and takes Level 1 by default because Level 2 is
still unstable around event order with composition events.

For reducing string copy cost at dispatching "input" event, this patch
makes EditorInternalInputEvent store valid input-type as enum class,
EditorInputType and resolves it to string value when
dom::InputEvent::GetInputType() is called.  Note that the reason why
this patch names the enum class as EditorInputType is, there is InputType
enum class already for avoiding conflict the name, this appends "Editor"
prefix because "input" and "beforeinput" events are fired only when an
editor has focus.

Differential Revision: https://phabricator.services.mozilla.com/D14128

--HG--
extra : moz-landing-system : lando
2019-01-07 10:10:57 +00:00
Emilio Cobos Álvarez
d2ed260822 Bug 1517241 - Rename nsIDocument to mozilla::dom::Document. r=smaug
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.

Overall it's not a very interesting patch I think.

nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.

I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.

While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
2019-01-03 17:48:33 +01:00
Sylvestre Ledru
265e672179 Bug 1511181 - Reformat everything to the Google coding style r=ehsan a=clang-format
# ignore-this-changeset

--HG--
extra : amend_source : 4d301d3b0b8711c4692392aa76088ba7fd7d1022
2018-11-30 11:46:48 +01:00
Emilio Cobos Álvarez
9ed36d7ba6 Bug 1506547 - Align user-select behavior more with other UAs. r=mats
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.

The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:

 * First, I don't think non-editable regions of an editable element should
   be user-select: all.

 * Second, it just doesn't work in Shadow DOM (the editable descendant count is
   not kept up-to-date when not in the uncomposed doc), so nested
   contenteditables behave differently inside vs. outside a Shadow Tree.

 * Third, I think it's user hostile to just entirely disable selection if you
   have a contenteditable descendant as a child of a user-select: all thing.

   WebKit behaves like this patch in the following test-case (though not Blink):

     https://crisal.io/tmp/user-select-all-contenteditable-descendant.html

   Edge doesn't seem to support user-select: all at all (no pun intended).

   But we don't allow to select anything at all which looks wrong.

 * Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
   and not even notice...).

In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.

The other change is `all` not overriding everything else. So, something like:

  <div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>

Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.

This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.

This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.

This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...

In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.

This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.

This also fixes issues with br-skipping like not being able to insert content in
the following test-case:

  <div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>

If you start moving to the left from the second line, for example.

I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.

There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.

Differential Revision: https://phabricator.services.mozilla.com/D12687

--HG--
extra : moz-landing-system : lando
2018-11-26 09:21:37 +00:00
Masayuki Nakano
b376e84323 Bug 1497746 - part 4: Move EditorBase::mRangeUpdater to AutoEditActionDataSetter r=m_kato
Similar to EditorBase::mSavedSel, we can move EditorBase::mRangeUpdater too
because of it's referred only when there is AutoEditActionDataSetter instance
so that it also does not need to be in the cycle collection.

And now, it can be marked as MOZ_STACK_CLASS and remove cycle collection
support.

Differential Revision: https://phabricator.services.mozilla.com/D12402

--HG--
extra : moz-landing-system : lando
2018-11-26 06:31:56 +00:00
Masayuki Nakano
692bb4e545 Bug 1497746 - part 3: Move EditorBase::mSavedSel into AutoEditActionDataSetter r=m_kato
EditorBase::mSavedSel is used only by EditorBase methods which are called only
by AutoSelectionRestorer.  Additionally, AutoSelectionRestorer requires
AutoEditActionDataSetter instance.  So, we don't need to keep create for
editor instance anymore.  And also we don't need to keep it in the cycle
collection.

Note that SelectionState class is also used by PlaceholderTransaction.
Therefore, we cannot make it MOZ_STACK_CLASS.

Differential Revision: https://phabricator.services.mozilla.com/D12401

--HG--
extra : moz-landing-system : lando
2018-11-26 06:31:13 +00:00
Masayuki Nakano
6daceee1f1 Bug 1497746 - part 2: Move EditorBase::mDirection to EditorBase::AutoEditActionDataSetter r=m_kato
EditorBase::mDirection is set and clear only when
EditorBase::AutoEditActionDataSetter::SetTopLevelEditSubAction().  So, the
direction is related to the top level edit sub action, and we can move it
into AutoEditActionDataSetter.

Note that except EditSubAction::eDeleteSelectedContent, the relation between
sub-action and direction is fixed so that this patch checks the relation with
MOZ_ASSERT.  If we could replace EditSubAction::eDeleteSelectedContent with
information of direction, we'd remove the new member of
AutoEditActionDataSetter, though.

Differential Revision: https://phabricator.services.mozilla.com/D12400

--HG--
extra : moz-landing-system : lando
2018-11-26 06:30:29 +00:00
Masayuki Nakano
49181b7721 Bug 1497746 - part 1: Move EditorBase::mTopLevelEditSubAction to EditorBase::AutoEditActionDataSetter r=m_kato
EditorBase::mTopLevelEditSubAction is set only by
EditorBase::OnStartToHandleTopLevelEditSubAction() and
EditorBase::OnEndToHandleTopLevelEditSubAction() and they are called only by
AutoTopLevelEditSubActionNotifier().

So, this is used only in stack when a public method of editor is called.
Therefore, we can move it into EditorBase::AutoEditActionDataSetter.  Then,
we can reduce heap allocation for editor instances.

Differential Revision: https://phabricator.services.mozilla.com/D12399

--HG--
extra : moz-landing-system : lando
2018-11-26 03:53:29 +00:00
Brindusan Cristian
31f0c21cca Backed out changeset 1575904619b5 (bug 1506547) for mochitest failures on test_reftests_with_caret.html. 2018-11-26 03:03:14 +02:00
Emilio Cobos Álvarez
b74c31e4d9 Bug 1506547 - Align user-select behavior more with other UAs. r=mats
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.

The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:

 * First, I don't think non-editable regions of an editable element should
   be user-select: all.

 * Second, it just doesn't work in Shadow DOM (the editable descendant count is
   not kept up-to-date when not in the uncomposed doc), so nested
   contenteditables behave differently inside vs. outside a Shadow Tree.

 * Third, I think it's user hostile to just entirely disable selection if you
   have a contenteditable descendant as a child of a user-select: all thing.

   WebKit behaves like this patch in the following test-case (though not Blink):

     https://crisal.io/tmp/user-select-all-contenteditable-descendant.html

   Edge doesn't seem to support user-select: all at all (no pun intended).

   But we don't allow to select anything at all which looks wrong.

 * Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
   and not even notice...).

In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.

The other change is `all` not overriding everything else. So, something like:

  <div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>

Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.

This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.

This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.

This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...

In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.

This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.

This also fixes issues with br-skipping like not being able to insert content in
the following test-case:

  <div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>

If you start moving to the left from the second line, for example.

I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.

There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.

Differential Revision: https://phabricator.services.mozilla.com/D12687

--HG--
extra : moz-landing-system : lando
2018-11-25 20:01:07 +00:00
Masayuki Nakano
abe138f771 Bug 1504911 - part 1: Make all "input" event dispatcher in C++ use new utility method r=smaug
Currently, a lot of code dispatch "input" event and some of them dispatch
"input" event with wrong interface and/or values.  Therefore this patch
creates nsContentUtils::DispatchInputEvent() to make all of them dispatch
correct event.

Unfortunately, due to bug 1506439, we cannot set pointer to refcountable
classes of MOZ_CAN_RUN_SCRIPT method to nullptr.  Therefore, this patch
creates temporary RefPtr<TextEditor> a lot even though it makes damage to
the performance if it's in a hot path.

This patch makes eEditorInput event dispatched with
InternalEditorInputEvent when "input" event should be dispatched with
dom::InputEvent.  However, this patch uses WidgetEvent whose message is
eUnidentifiedEvent and setting WidgetEvent::mSpecifiedEventType to
nsGkAtoms::oninput when "input" event should be dispatched with
dom::Event because we need to keep that eEditorInput and
InternalEditorInputEvent are mapped each other.

Differential Revision: https://phabricator.services.mozilla.com/D12244

--HG--
extra : moz-landing-system : lando
2018-11-21 03:59:02 +00:00
Masayuki Nakano
b5f863bf51 Bug 1504131 - part 1: Remove DocumentResizeEventListener r=m_kato
DocumentResizeEventListener listens to only "resize" events of the window
and when it fired, it just calls HTMLEditor::RefreshResizers().  Fortunately,
neither EditorEventListener nor HTMLEditorEventListener listens to "resize"
events.  Therefore, we can move this implementation into
HTMLEditorEventListener.

Differential Revision: https://phabricator.services.mozilla.com/D10866

--HG--
extra : moz-landing-system : lando
2018-11-06 04:58:29 +00:00
Masayuki Nakano
d97102c926 Bug 1465702 - part 7: Make protected/private methods of EditorBase/TextEditor/HTMLEditor use SelectionRefPtr() instead of GetSelection() r=m_kato
Now, any protected/private methods of editor classes can refer
SelectionRefPtr() safely.  So, we can cut the cost of calling GetSelection()
only once per edit action handling.

Differential Revision: https://phabricator.services.mozilla.com/D10011

--HG--
extra : moz-landing-system : lando
2018-10-30 10:04:08 +00:00
Masayuki Nakano
53578eb0db Bug 1465702 - part 6: Remove unnecessary Selection argument from TextEditRules and HTMLEditRules and rename/reimplement TextEditRules::SelectionRef() as TextEditRules::SelectionRefPtr() r=m_kato
When each method of TextEditRules and HTMLEditRules should be called only by
EditorBase/TextEditor/HTMLEditor while the editor instance has
AutoEditActionDataSetter instance.  Therefore, we can get rid of all
pointers/references of Selection from each method's argument.

Additionally, this reimplements TextEditRules::SelectionRef() with
EditorBase::SelectionRefPtr() and make it return |const RefPtr<Selection>&|
rather than |Selection&| since RefPtr reference is required when we call
methods which are marked as MOZ_CAN_RUN_SCRIPT.

Differential Revision: https://phabricator.services.mozilla.com/D10010

--HG--
extra : moz-landing-system : lando
2018-10-30 10:02:58 +00:00
Masayuki Nakano
f91716e775 Bug 1465702 - part 5: Remove unnecessary Selection argument from editor module r=m_kato
EditorBase::SelectionRefPtr() is now safe to use in editor and really fast to
retrieve Selection than EditorBase::GetSelection().  Therefore, we can get rid of
all Selection pointer/reference argument of each method which always take
normal Selection.

Note that this changes nsIHTMLEditor.checkSelectionStateForAnonymousButtons()
because its argument is only Selection.  So, BlueGriffon should work even
though it calls the method with nsIEditor.selection.

Differential Revision: https://phabricator.services.mozilla.com/D10009

--HG--
extra : moz-landing-system : lando
2018-10-30 10:01:38 +00:00
Masayuki Nakano
779965a889 Bug 1465702 - part 2: Make public methods of EditorBase create AutoEditActionDataSetter if necessary r=m_kato
This patch makes public methods of EditorBase create AutoEditActionDataSetter
as far as possible.  However, does not do so for some public methods if they are
not necessary to create it.

Differential Revision: https://phabricator.services.mozilla.com/D10006

--HG--
extra : moz-landing-system : lando
2018-10-30 09:57:52 +00:00
Masayuki Nakano
7f8df77fb6 Bug 1465702 - part 1: Add EditorBase::AutoEditActionDataSetter to store edit action and grab Selection instance while editor classes handle an edit action r=m_kato
Like TextEditRules, EditorBase should have a stack class which cache necessary
objects and current handling edit action.  The edit action will be necessary
when we implement InputEvent.inputType.

Different from TextEditRules, this adds |const RefPtr<Selection>&| instead
of |Selection&|.  The reason is, when I add MOZ_CAN_RUN_SCRIPT to some methods,
it's not allowed like this:

> foo->CanRunScriptMethod(SelectionRef());

I'll update TextEditRules for consistency in the following patches.

Differential Revision: https://phabricator.services.mozilla.com/D10005

--HG--
extra : moz-landing-system : lando
2018-10-30 09:56:41 +00:00
Makoto Kato
0ca1f40c1b Bug 1485890 - Remove dumpContentTree, debugDumpContent and debugUnitTests from nsIEditor. r=masayuki
Since bug 1491173 is landed, there is no users of dumpContentTree,
debugDumpContent and debugUnitTests.  So Let's remove these methods from
nsIEditor.

Differential Revision: https://phabricator.services.mozilla.com/D10029

--HG--
extra : moz-landing-system : lando
2018-10-29 07:23:49 +00:00
Masayuki Nakano
cb95d44ffc Bug 1500862 - part 3: Change all stack base helper classes which access protected members of EditorBase to nested classes of EditorBase r=m_kato
AutoTransactionBatch, AutoPlaceholderBatch, AutoSelectionRestorer,
AutoTopLevelEditSubActionNotifier, AutoTransactionsConserveSelection and
AutoUpdateViewBatch access protected members of EditorBase.  The access
scope management assume that they are used only by EditorBase or its
subclasses or TextEditRules or its inherited class (i.e., HTMLEditRules).

For guaranteeing this at build-time, we should change them to nested classes
of EditorBase.  Then, EditorBase and its subclasses and friends can use them.

Differential Revision: https://phabricator.services.mozilla.com/D9479

--HG--
extra : moz-landing-system : lando
2018-10-24 09:42:06 +00:00
Masayuki Nakano
9b40433ef6 Bug 1461708 - part 7: Make EventStateManager::HandleMiddleClickPaste() dispatch ePaste event by itself r=smaug
This is preparation of the last patch.  Even if no editor is clicked with
middle button, we need to do:
- collapse Selection at the clicked point.
- dispatch "paste" event.

Therefore, HandleMiddleClickPaste() should dispatch ePaste event by itself
and each editor methods should have a bool argument which the caller wants
ePaste event automatically.

Note that Chromium dispatches "paste" event and pastes clipboard content
into clicked editor even if preceding "auxclick" event is consumed.
However, our traditional behavior is not dispatching "paste" event nor
pasting clipboard content.  Unless Chromium developer keeps their odd
behavior, we should keep our traditional behavior since our behavior is
conforming to DOM event model.

Differential Revision: https://phabricator.services.mozilla.com/D7854

--HG--
extra : moz-landing-system : lando
2018-10-10 12:05:39 +00:00
Makoto Kato
eed71e1db2 Bug 1487301 - Part 1. FindSelectionRoot should return Element. r=masayuki
FindSelectionRoot isn't const method and returns already_AddRefed<nsIContent>.
But this method doesn't modify any members and nodes, so we can change to const
method

Also, this method already returns Element, so it shouldn't return nsIContent.

Differential Revision: https://phabricator.services.mozilla.com/D6373

--HG--
extra : rebase_source : d4a5dbe96dfc0a71b39f3d5c6d1a4c7ce85f4fa9
2018-09-20 18:53:35 +09:00
Andrew McCreight
837f0af066 Bug 1493737 - Fix many trivial calls to do_QueryInterface r=smaug
If class A is derived from class B, then an instance of class A can be
converted to B via a static cast, so a slower QI is not needed.

Differential Revision: https://phabricator.services.mozilla.com/D6861

--HG--
extra : moz-landing-system : lando
2018-10-01 21:38:01 +00:00
Masayuki Nakano
7fc2bdaae1 Bug 1484110 - part 1: Create HTMLEditor::RefereshEditingUI() for internal use of nsIHTMLEditor::CheckSelectionStateForAnonymousButtons() r=m_kato
HTMLEditor::CheckSelectionStateForAnonymousButtons() is called a lot internally.
Especially, its virtual call cost may make damage to our performance since
it's called from a selection listener.

So, we should create non-virtual method, RefereshEditingUI() for internal use.
2018-08-17 17:56:28 +09:00
Masayuki Nakano
4bf13b90c7 Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r=m_kato
EditorBase::AreNodesSameType() is overridden only by HTMLEditor and the
implementation is enough simple to re-implement in EditorBase.

Additionally, this is called from condition of a loop in
JoinNodesDeepWithTransaction().  So, the virtual call cost may make damage
to the performance.

Differential Revision: https://phabricator.services.mozilla.com/D3460

--HG--
extra : moz-landing-system : lando
2018-08-16 10:29:20 +00:00
Masayuki Nakano
b8fd63d7b5 Bug 1482020 - Make all callers of CreateElementWithDefaults() use non-virtual method r=m_kato
Fortunately, despite of becoming public method,
HTMLEditor::CreateElementWithDefaults() can be used by internal methods too
since it does not touch undo transactions nor the DOM tree, and does not
refer mRules nor GetSelection().  So, we can make it public and make any
C++ callers use it.
2018-08-10 19:36:24 +09:00
Masayuki Nakano
c0e869978e Bug 1482012 - part 2: Create TextEditor::PasteAsAction() as non-virtual method for outer C++ code r=m_kato
User may paste a lot with pressing Accel+V for a while (i.e., with auto repeat).
So, calling nsIEditor::Paste() may be in a hot path and we can now make
non-virtual public method with AsHTMLEditor().

Differential Revision: https://phabricator.services.mozilla.com/D2993

--HG--
extra : moz-landing-system : lando
2018-08-13 04:37:56 +00:00
Masayuki Nakano
89e5796790 Bug 1482007 - part 1: Create a helper class to guarantee to call nsIEditor::EndTransaction() after nsIEditor::BeginTransaction() call r=m_kato
This patch also creates non-virtual methods,
EditorBase::BeginTransactionInternal(), EditorBase::EndTransactionInternal(),
TransactionManager::BeginBatchInternal() and
TransactionManager::EndBatchInternal().

Although, this could be replaced with API to use PlaceholderTransaction,
we should investigate it when we have much time.

Differential Revision: https://phabricator.services.mozilla.com/D2989

--HG--
extra : moz-landing-system : lando
2018-08-09 11:45:41 +00:00
Emilio Cobos Álvarez
1a7f1a4dbf Bug 1478604 - Fix RepaintSelectionRunner so that it does something for non-presShell impls. r=masayuki
The regressing bug made RepaintSelectionRunner do nothing for any
nsISelectionListener that wasn't a PresShell.

Differential Revision: https://phabricator.services.mozilla.com/D2846

--HG--
extra : moz-landing-system : lando
2018-08-07 12:55:21 +00:00