The .cpp files under editor/txmgr have a lot of unnecessary empty lines and they make reading the code more difficult because fewer lines can be displayed in screen. This patch removes such unnecessary lines and C like variable declarations at start of some methods because they also waste lines and make them unclear where is the first user of them.
MozReview-Commit-ID: GXGkdoLJ4Jw
--HG--
extra : rebase_source : 449e191553cff8c875565802d930abe2a6dba4a6
For conforming to our coding rules, |result| of nsresult variants should be renamed to |rv|.
MozReview-Commit-ID: Bk8CyLAnvXQ
--HG--
extra : rebase_source : dd3dc34a032f22abf3fd7f85556b47ffbeec55b2
SetDocumentTitleTransaction.cpp was written a long time ago. So, it doesn't use our modern coding style. Let's fix it.
MozReview-Commit-ID: LhgMbv8dqKm
When there is no title element, SetDocumentTitleTransaction::SetDomTitle() creates title element and try to insert it to the head element. However, it might be swapped during modifying existing title element or head element.
Therefore, after modifying the tree, SetDocumentTitleTransaction::SetDomTitle() should forget the retrieved head element and when it needs to refer current head element, it should retry to get the head element again.
MozReview-Commit-ID: 59BxEwrflye
For detecting regressions, SetDocumentTitleTransaction::SetDomTitle() should be tested.
The transaction is created only when nsIHTMLEditor::setDocumentTitle(). Therefore, it needs to be a chrome mochitest.
MozReview-Commit-ID: 4UjAJ8zNPBP
Currently, editor code uses following style (or similar style) in a lot of places:
if (foo)
{
}
else
{
}
This patch fixes this as conforming to our coding rules, i.e., it becomes:
if (foo) {
} else {
}
Additionally, this fixes other odd control statements in the files which include above issue because it's difficult to find following issues with searching the files:
* if (foo) bar;
* if (foo) { bar; }
* if (foo)
bar;
Finally, if it becomes much simpler than current code, this patch rewrites existing code with "early return style". But this case is only a few places because this is risky.
MozReview-Commit-ID: 2Gs26goWXrF
--HG--
extra : rebase_source : 603f9003a3566b3203bdeb27dc73ac33502d2853
In our coding rules, variable names of nsresult should be rv. Indeed, when you see |rv| in the code, you must assume that its type if nsresult.
However, a lot of code under editor/ uses |res| for the variables of nsresult. Let's replace |res| with |rv|.
And this patch improves following points:
1. When |rv| is set in both |if| and |else| block and they are check outside of them, this moves the check into each |if| and |else| block because even if the failure is notified with warning, you cannot see which case was performed and failed. This change makes it clear.
2. When |return rv;| returns non-error code because |rv| is checked with NS_ENSURE_SUCCESS() immediately before, setting replacing it with |return NS_OK;| is clearer.
3. Move declaration of |nsresult rv| into smaller scope as far as possible. This prevents setting rv to unexpected value and easier to check its value at reading the code.
MozReview-Commit-ID: 9MAqj7sFey3
--HG--
extra : rebase_source : 0fd316b851ea616b3a95d8c1afc111ff55e11993
NS_FOUND_TARGET is now declared as an error code. However, making it as a success code makes the code simpler.
First, this patch renames it to NS_SUCCESS_EDITOR_FOUND_TARGET because it's usual naming rule (according to the other declarations).
Next, FindTargetNode() should return it when a nest call of itself returns NS_SUCCESS_EDITOR_FOUND_TARGET as it does now.
Finally, removing the code overwriting NS_FOUND_TARGET with NS_OK from HTMLEditor::CreateDOMFragmentFromPaste() since it doesn't cause hitting NS_ENSURE_SUCCESS() and the variable, rv, will be overwritten with other method's result.
MozReview-Commit-ID: 6GgZptrXXQa
--HG--
extra : rebase_source : 79418fc83fb087e559221f895c59c7c334980456
When typing character, current selection node might be anonymous DIV, not text node. So even if plain text, we might not get it.
We should get text node correctly when using plain text editor.
MozReview-Commit-ID: LmfYa7BqZnC
--HG--
extra : rebase_source : d08c74c8cc5fdec4d19772f112e54a08b95afeb3
When pasting large image, it may cause OOM when generating base64 data. So we should use fallible allocator instead.
Also, base64 is ASCII, so we should use AppendASCIItoUTF16 instead of AppendUTF8toUTF16.
MozReview-Commit-ID: 8yWPxfEcEwv
--HG--
extra : rebase_source : 6c2c3144d4413017b4861443c945aeb0d6d995ef
We can assume that if middle button's click event on a link isn't consumed by any event handlers including system event group's, it will cause open new tab. With this assumption, we can avoid using setTimeout which causes random orange.
However, unfortunately, in e10s mode, the default is NOT consumed at window in bubbling phase but consumed at that time. So, when not working the link is expected, we cannot check Event.defaultPrevented. But fortunately, we can check if the page is loaded after that.
Note that for testing this, the test needs to check if an event handler which is either in default group or system group consumed a click event. However, this runs as mochitest-plain. Therefore, Event.defaultPrevented returns false if the event is consumed only in the system group's event listener. For avoiding this issue, this patch adds defaultPreventedInAnyGroups() into SpecialPowers. In SpecialPowers, Event.defaultPrevented is accessed from chrome context. Therefore, we can get the result what this test needs.
MozReview-Commit-ID: Cfn4lFR1dfI
--HG--
extra : rebase_source : 51feb768bd38f62cc19c2f4aecaaea0135190599
The new name makes the sense of the condition much clearer. E.g. compare:
NS_WARN_IF_FALSE(!rv.Failed());
with:
NS_WARNING_ASSERTION(!rv.Failed());
The new name also makes it clearer that it only has effect in debug builds,
because that's standard for assertions.
--HG--
extra : rebase_source : 886e57a9e433e0cb6ed635cc075b34b7ebf81853
The main renaming was generated with the following python script:
```
import sys
import re
CAMEL_CASE_REGEX = re.compile(r"(^|_|-)([A-Z])([A-Z]+)")
DISPLAY_REGEX = re.compile(r"\bNS_STYLE_DISPLAY_([^M][A-Z_]+)\b")
def to_camel_case(ident):
return re.sub(CAMEL_CASE_REGEX,
lambda m: m.group(2) + m.group(3).lower(), ident)
def constant_to_enum(constant):
return "StyleDisplay::" + to_camel_case(constant) + ("_" if constant == "NONE" else "")
def process_line(line):
return re.sub(DISPLAY_REGEX,
lambda m: constant_to_enum(m.group(1)), line)
lines = []
with open(sys.argv[1], "r") as f:
for line in f:
lines.append(process_line(line))
with open(sys.argv[1], "w") as f:
for line in lines:
f.write(line)
```
And the following shell commands:
```
find . -name '*.cpp' -exec python display.py {} \;
find . -name '*.h' -exec python display.py {} \;
```
MozReview-Commit-ID: 91xYCbLC2Vf
They are non-standard aliases for "DragEvent" and "KeyboardEvent" that
are not supported by any other UA, and we would like to drop support.
So first let's stop using them ourselves, so we can use telemetry to see
if any sites are using them.
MozReview-Commit-ID: ICC33ORa2st
CompositionTransaction::SetIMESelection() is also called when user redos the committed composition. In such case, it doesn't have ranges, but DoTransaction() tries to restore selection ranges with no ranges. That causes hiding caret because when there is no caret range in the ranges, SetIMESelection() hides the caret as IME requested.
So, for redo, SetIMESelection() shouldn't hide caret when there are no ranges (i.e., the composition was already committed).
Previously we supported insertParagraph as a synonym for formatBlock
"p", which is both useless and incompatible with all other browsers.
Edge's behavior doesn't look useful and looks redundant with insertHTML
(although it matches the name nicely, probably because they invented
the command). Blink/WebKit treat it the same as hitting Enter, which is
useful for cross-browser testing until we get synthesizeKey() in wpt.
At the same time, I added insertLineBreak, which Blink/WebKit support
with the same functionality as pressing Shift-Enter.
The new event.html failures are spurious -- we used to pass by mistake
because we didn't support insertlinebreak at all. insertlinebreak.html
has only new passes, no new failures, although it's not clear on the
diff.
MozReview-Commit-ID: 16oPcxXwGcj
Many HTMLEditRules methods use the pattern of
nsCOMPtr<nsIEditor> kungFuDeathGrip(mHTMLEditor);
at the beginning of the method, followed by using mHTMLEditor freely.
If Init() is then called on the editor, mHTMLEditor is set to null, so
it will crash. Other bad stuff will probably happen too.
MozReview-Commit-ID: gUtaTAQJIh
When deleting, we previously would only move the cursor back into the
previous block if the last leaf node was a text node, for some reason.
We should move into the previous block in other cases as well, like if
the leaf node is a collapsed <br>. (Probably it's not correct to move
backward into tables, but that already was happening if the table had
text at the end. There may be other cases where this is wrong.)
MozReview-Commit-ID: 8e0dTU3lNYO
If the user tries to insert text without moving the cursor, the
invisible break will become visible, which from the user's perspective
means an extra line break was inserted for no reason.
The IsVisBreak change here is a bit scary, because it may result in
unexpected behavior changes, but it seems to improve correctness. If it
turns out to cause problems, it can be added as a special case to
GetGoodSelPointForNode instead.
MozReview-Commit-ID: 7SRQYHYtJvF
When deleting, we previously would only move the cursor back into the
previous block if the last leaf node was a text node, for some reason.
We should move into the previous block in other cases as well, like if
the leaf node is a collapsed <br>. (Probably it's not correct to move
backward into tables, but that already was happening if the table had
text at the end. There may be other cases where this is wrong.)
MozReview-Commit-ID: 8e0dTU3lNYO
If the user tries to insert text without moving the cursor, the
invisible break will become visible, which from the user's perspective
means an extra line break was inserted for no reason.
The IsVisBreak change here is a bit scary, because it may result in
unexpected behavior changes, but it seems to improve correctness. If it
turns out to cause problems, it can be added as a special case to
GetGoodSelPointForNode instead.
MozReview-Commit-ID: 7SRQYHYtJvF
Bug 1268736 inadvertently put a new test before a skip-if line, which
makes the new test skipped instead of the preexisting one.
MozReview-Commit-ID: 20SuUc3IhiG
This patch is generated by the following commands (note: if you're running
using OS X's sed, which accepts slightly different flags, you'll have to
specify an actual backup suffix in -i, or use gsed from Homebrew):
hg stat -c \
| cut -c 3- \
| tr '\n' '\0' \
| xargs -0 -P 8 gsed --follow-symlinks 's/\bnsCSSProperty\b/nsCSSPropertyID/g' -i''
Then:
hg mv layout/style/nsCSSProperty.h layout/style/nsCSSPropertyID.h
... and finally, manually renaming nsCSSProperty in the include guard in
nsCSSProperty.h.
MozReview-Commit-ID: ZV6jyvmLfA
--HG--
rename : layout/style/nsCSSProperty.h => layout/style/nsCSSPropertyID.h
The merge from inbound to central conflicted with the merge from
autoland to central, it appears. Per tree rules, the commit from the
autoland repo wins and the inbound commit gets backed out.
CLOSED TREE
--HG--
extra : amend_source : 927e1cdfa8e55ccbd873d404d905caf6871c8c4f
extra : histedit_source : 07095868c3f767258e1d7d2645193bf4811b13bb%2Ca49ae5a28bf6e67298b6208ee9254c25a2539712
This patch is generated by the following commands (note: if you're running
using OS X's sed, which accepts slightly different flags, you'll have to
specify an actual backup suffix in -i, or use gsed from Homebrew):
hg stat -c \
| cut -c 3- \
| tr '\n' '\0' \
| xargs -0 -P 8 gsed --follow-symlinks 's/\bnsCSSProperty\b/nsCSSPropertyID/g' -i''
Then:
hg mv layout/style/nsCSSProperty.h layout/style/nsCSSPropertyID.h
... and finally, manually renaming nsCSSProperty in the include guard in
nsCSSProperty.h.
MozReview-Commit-ID: ZV6jyvmLfA
--HG--
rename : layout/style/nsCSSProperty.h => layout/style/nsCSSPropertyID.h
This patch makes most Run() declarations in subclasses of nsIRunnable have the
same form: |NS_IMETHOD Run() override|.
As a result of these changes, I had to add |override| to a couple of other
functions to satisfy clang's -Winconsistent-missing-override warning.
--HG--
extra : rebase_source : 815d0018b0b13329bb5698c410f500dddcc3ee12
nsIContent::GetDesiredIMEState sets IMEState::DISABLED, so IME doesn't work well.
Actually, DetachEditorFromWindow is called twice. One is from nsEditingSession::StartDocumentLoad, and another is from nsDocShell::FirePageHideNotification.
When navigating to next, OnStateChange(..., STATE_START) is called. Even if next page is external viewer such as launching another application, STAT_START is called. So even if launching external application, DetachEditorFromWindow is called. So it means that editor is detached from docshell.
nsIContent::GetDesiredIMEState uses nsIEditor, but editor is detached state, so it cannot get nsIEditor.
When navigating to next content on internel viewer such HTML, nsDocShell::CreateContentViewer is called, then FirePageHideNotification is called.
So it is unnecessary to call DetachEditorFromWindow from nsEditorSession. And its call causes this issue.
MozReview-Commit-ID: Lf5Rhzm1PNF
--HG--
extra : rebase_source : c452e9ea0fc01670b49cd76e17fb782aef3a5c20
This removes the unnecessary setting of c-basic-offset from all
python-mode files.
This was automatically generated using
perl -pi -e 's/; *c-basic-offset: *[0-9]+//'
... on the affected files.
The bulk of these files are moz.build files but there a few others as
well.
MozReview-Commit-ID: 2pPf3DEiZqx
--HG--
extra : rebase_source : 0a7dcac80b924174a2c429b093791148ea6ac204
This patch also renames:
EditorInputEventDispatcher -> mozilla::EditorInputEventDispatcher
And some variable names are renamed from aEditor or mEditor to aEditorBase or mEditorBase for making their types clearer.
MozReview-Commit-ID: 2FCXWpLMn8e
--HG--
rename : editor/libeditor/nsEditor.cpp => editor/libeditor/EditorBase.cpp
rename : editor/libeditor/nsEditor.h => editor/libeditor/EditorBase.h
This patch renames:
ResizerSelectionListener -> mozilla::ResizerSelectionListener
ResizerMouseMotionListener -> mozilla::ResizerMouseMotionListener
DocumentResizeEventListener -> mozilla::DocumentResizeEventListener
And making the header file name HTMLEditorObjectResizerUtils.h because it doesn't define specific class related to its file name and should be clearer that it's related to HTMLEditor.
MozReview-Commit-ID: GOJyEwtFdJV
--HG--
rename : editor/libeditor/nsHTMLObjectResizer.h => editor/libeditor/HTMLEditorObjectResizerUtils.h
Perhaps, there may be better name like WhitespaceRunObject or something, however, for now keep using the term because I don't understand well what it does.
With this patch, following objects are renamed:
nsWSRunObject -> mozilla::WSRunObject
WSType -> mozilla::WSType
nsWSRunObject::WSFragment -> mozilla::WSRunObject::WSFragment
nsWSRunObject::WSPoint -> mozilla::WSRunObject::WSPoint
MozReview-Commit-ID: JgAWiPjOtMW
--HG--
rename : editor/libeditor/nsWSRunObject.cpp => editor/libeditor/WSRunObject.cpp
rename : editor/libeditor/nsWSRunObject.h => editor/libeditor/WSRunObject.h
This patch also fixes new bustage of nsHTMLEditRules.cpp and nsWSRunObject.cpp.
MozReview-Commit-ID: 5xgZDP7d6wy
--HG--
rename : editor/libeditor/nsHTMLURIRefObject.cpp => editor/libeditor/HTMLURIRefObject.cpp
rename : editor/libeditor/nsHTMLURIRefObject.h => editor/libeditor/HTMLURIRefObject.h
Note that this fixes some new bustage of nsHTMLEditor.
nbsp is conflict with nsWSRunObject.cpp's same name constant. Therefore, I moved it into nsHTMLEditor and rename it to kNBSP.
And including some missing header files.
MozReview-Commit-ID: hwL8BIbSMQ
--HG--
rename : editor/libeditor/nsSelectionState.cpp => editor/libeditor/SelectionState.cpp
rename : editor/libeditor/nsSelectionState.h => editor/libeditor/SelectionState.h
Moving PropItem and TypeInState into mozilla namespace. I.e., these names are:
PropItem -> mozilla::PropItem
TypeInState -> mozilla::TypeInState
Although, there might be better names for them. They are too general names.
MozReview-Commit-ID: By9CX1KlAaU
This patch also moves ImplCycleCollectionTraverse() and ImplCycleCollectionUnlink() to mozilla namespace for avoiding bustage due to confusion caused by "using namespace" in other unified cpp files.
MozReview-Commit-ID: GekPlZnqLs9
--HG--
rename : editor/libeditor/nsStyleSheetTxns.cpp => editor/libeditor/StyleSheetTransactions.cpp
rename : editor/libeditor/nsStyleSheetTxns.h => editor/libeditor/StyleSheetTransactions.h
This patch renames IMETextTxn to CompositionTransaction. "Composition" is now used in some web standard specs, e.g., CompositionEvent defined by UI Events.
This patch also renames nsEditor::CreateTxnForIMEText() to nsEditor::CreateTxnForComposition().
MozReview-Commit-ID: JN0GgGdpAWG
--HG--
rename : editor/libeditor/IMETextTxn.cpp => editor/libeditor/CompositionTransaction.cpp
rename : editor/libeditor/IMETextTxn.h => editor/libeditor/CompositionTransaction.h
Failure to increment the refcount to this BR node causes fatal cycle
collector assertions, and could lead to all sorts of other crashes.
This is a regression from bug 1156062, part 5.
This patch defines two constants kSelectionTypeCount and kPresentSelectionTypeCount. The former is same as nsISelectionController::NUM_SELECTIONTYPES. The latter is kSelectionTypeCount - 1 for excluding SELECTION_NONE. The latter is useful in some loops which handle all selection types except SELECTION_NONE.
Note that this patch fixes a bug of nsFrameSelection. That doesn't treat SELECTION_NONE as a selection (see the definition of index), however, it defines redundant item and doesn't use it actually. Additionally, it computes invalid selection type in each loop. Therefore, without this patch, debug build hits MOZ_ASSERT() in ToSelectionType(RawSelectionType).
Note that these constants are defined as anonymous enum because we cannot define as const (or static) even with extern. If we'd try to do it, it caused link error or not available in nsFrameSelection.cpp as constant value since they were not initialized if they were initialized in nsSelection.cpp. Therefore, these constants are defined as enum items but using "k" prefix.
MozReview-Commit-ID: H6sH7NBEXlE
--HG--
extra : rebase_source : fd517d5fc2e2d5dc2f96313e2802fd1719817af7
This patch defines mozilla::SelectionType as an enum class. This is safer than nsISelectionController::SELECTION_* since setting illegal value to its variable is checked at build time. So, as far as possible, this should be used everywhere (but of course, this isn't available in scriptable interfaces).
And also this implements some useful methods for managing SelectionType and RawSelectionType which are implemented in layout/nsSelection.cpp because nsISelectionController is implemented by both PresShell and nsTextEditorState. Therefore, implementing one of them may make hard to find them. On the other hand, nsSelection.cpp is a better file name to look for them.
Note that this patch creates mozilla::Selection::RawType() for binding. Native code should keep using Selection::Type() but the binding code needs to use RawType() due to impossible to convert from SelectionType to RawSelectionType without explicit cast.
MozReview-Commit-ID: 81vX7A0hHQN
--HG--
extra : rebase_source : d9f88e217c713c60d1c2578ce6421c73ccba8650
mozilla::SelectionType will be an enum class. Therefore, we need to rename SelectionType with a word "raw" since it's a type for raw nsISelectionController::SELECTION_*.
MozReview-Commit-ID: K8SO0bbpv0Y
--HG--
extra : rebase_source : 56e3ac852657ef4795d718d2da09b16903c2e66c
On content sandbox process, GetKeyboardState() API doesn't return current keyboard state. So we shouldn't use it.
MozReview-Commit-ID: 4phnJf0sJFZ
--HG--
extra : rebase_source : ea52839d427431a2ba7624ef199f9842fe12f8ef
extra : histedit_source : f7905715eb4e09a0234b9f7717cec6161150a7f3
For making our code clearer by the stronger type check, we should change the anonymous enum for NS_TEXTRANGE_* to enum class whose name is "TextRangeType" and whose type is "RawTextRangeType" which is an alias of uint8_t.
Additionally, this also adds some utility methods for them.
Note that some lines which are changed by this patch become over 80 characters but it will be fixed by the following patches.
MozReview-Commit-ID: 76izA1WqTkp
--HG--
extra : rebase_source : 27cd8cc8f7f8e82055dbfe82aba94c02beda5fa4
nsHTMLEditRules::ReapplyCachedStyles() may be called after the document is removed from the DOM tree. For example, the document can be removed from the tree even during handling an edit operation if the web contents uses DOMMutationEvent. In such case, nsIEditor::GetSelection() returns nullptr and it should do nothing.
MozReview-Commit-ID: Jk3eGalAPhB
--HG--
extra : rebase_source : 54b8eb0ace2c656a68cd8aaf0edd3d1177b127ff
And mCharCode shouldn't be compared with NS_VK_*, nsIDOMKeyEvent::DOM_VK_*. Additionally, when it's compared with a character constant, cast isn't necessary.
MozReview-Commit-ID: JMT614copjG
--HG--
extra : rebase_source : 69ee3c589e5a71c814ec9a40ac3aab39c789c11d
And also WidgetKeyboardEvent::mKeyCode should be compared with NS_VK_* rather than nsIDOMKeyEvent::DOM_VK_*.
MozReview-Commit-ID: IKjQ1nr8XYe
--HG--
extra : rebase_source : 83125cd2523f6b70759f621470aad23b00aae8ae
This patch was generated by the command:
find . -name "*.css" -exec sed -i -f mozpropsub {} \;
in the root of a mozilla-central tree, with the file mozpropsub
containing the contents:
s/-moz-padding-end\>/padding-inline-end/g
s/-moz-padding-start\>/padding-inline-start/g
s/-moz-margin-end\>/margin-inline-end/g
s/-moz-margin-start\>/margin-inline-start/g
s/-moz-border-end\>/border-inline-end/g
s/-moz-border-end-color\>/border-inline-end-color/g
s/-moz-border-end-style\>/border-inline-end-style/g
s/-moz-border-end-width\>/border-inline-end-width/g
s/-moz-border-start\>/border-inline-start/g
s/-moz-border-start-color\>/border-inline-start-color/g
s/-moz-border-start-style\>/border-inline-start-style/g
s/-moz-border-start-width\>/border-inline-start-width/g
While I didn't manually review all the changes, I did review the list of
files, and manually reviewed the changes in the files that I thought
were more interesting.
Note that there are a few tests that should be fixed up as well, but
I'll do that in a later patch.
MozReview-Commit-ID: EiQTuuV0MNQ
This patch tells all callers to use the existing behavior, so it is
intended not to change behavior. Callers that will be modified in later
patches are marked with "FIXME" comments that will be removed in those
later patches (patches 3 and 4).
MozReview-Commit-ID: FaLryfxaeHv
The pref had been removed in bug 1249201. Need to clean it up for IMETextTxn.
MozReview-Commit-ID: EaFaN1yzoyP
--HG--
extra : rebase_source : 55da9499245d4d773bbce74e99c013f9814084b4
It looks like VC++ doesn't like comparisons of nsCOMPtr to 0 after this
change, but those are bad style anyway, so I removed them from
TestCOMPtr.cpp instead of trying to make them work.
