This patch changes remaining things under `layout/`. However, there are some
places which still need to use `nsIPresShell`. That will be fixed in a
follow up bug.
Differential Revision: https://phabricator.services.mozilla.com/D27477
--HG--
extra : moz-landing-system : lando
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
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
This way we don't have to deal with QI to get a Selection out of a weakref.
mfbt weakrefs don't have a SizeOfOnlyThis. In any case, the memory used by the
weakref itself is pretty minor...
This patch was generated automatically by the "modeline.py" script, available
here: https://github.com/amccreight/moz-source-tools/blob/master/modeline.py
For every file that is modified in this patch, the changes are as follows:
(1) The patch changes the file to use the exact C++ mode lines from the
Mozilla coding style guide, available here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Mode_Line
(2) The patch deletes any blank lines between the mode line & the MPL
boilerplate comment.
(3) If the file previously had the mode lines and MPL boilerplate in a
single contiguous C++ comment, then the patch splits them into
separate C++ comments, to match the boilerplate in the coding style.
MozReview-Commit-ID: EuRsDue63tK
--HG--
extra : rebase_source : 3356d4b80ff6213935192e87cdbc9103fec6084c
The bulk of this commit was generated by running:
run-clang-tidy.py \
-checks='-*,llvm-namespace-comment' \
-header-filter=^/.../mozilla-central/.* \
-fix
There's no need for this method to turn off blinking anymore. Its only
caller already calls SetCaretReadOnly to achieve the same effect. That
means we don't actually need the mIsBlinking flag after all.
--HG--
extra : rebase_source : 9d4c31282ed280c0f822f1d9f7fa8ae1c2ba6cab
A few things got mashed together here:
-- Inline KillTimer/PrimeTimer into their callers.
-- Instead of having to call StopBlinking and StartBlinking together,
change StartBlinking to ResetBlinking and have it set up
the correct blink state and reset the blink cycle.
-- nsCaret::NotifySelectionChange needs a SchedulePaint
-- nsCaret::DrawAtPosition needs a ResetBlinking
--HG--
extra : rebase_source : abc7fd78c4f20b787b212e1e3f13226a1ccff16b
This is the core of the whole patch set.
Now GetPaintGeometry/PaintCaret figure out on their own almost all the state
they need every time we paint. So when caret flags change, all we need
to do is SchedulePaint. We don't need to fiddle with mDrawn and most of the
logic in DrawCaret is obsolete. (In fact, it was duplicated by GetGeometry
and friends, and we're removing that duplication.) EraseCaret, CheckCaretState
and UpdateCaretPosition are also obsolete.
We need to have GetPaintGeometry/PaintCaret choose the correct content node
and offset, either getting them from the Selection or using specific data set by
DrawAtPosition. This logic, plus a bit of other code shared between them, is
put into the helper GetFrameAndOffset.
--HG--
extra : rebase_source : e777605dd2507ae043e9f82d0a30e23aa06e0c12
Instead of checking the mysterious mDrawn state (which is evil and will be
removed), let nsCaret::GetPaintGeometry take sole responsibilty for deciding
whether to draw. It takes the nsStyleUserInterface checks. It also needs to
check blink state, which is made possible by separating blink state into
the mIsBlinkOn flag.
--HG--
extra : rebase_source : 4a4796c37bc9ec7c25ffb2a320f9484cee1dc52f
This also fixes what appears to be a bug. MustDrawCaret returned true
when mShowDuringSelection is set even if the caret would otherwise be
hidden due a popup showing. That doesn't make sense.
--HG--
extra : rebase_source : e05e0892a85448dbc6666e23a0dbc7fa21a9c61e
This code is somewhat tricky. nsCaret::ComputeCaretRects was checking to see
if we have a bidi keyboard, and if so, what direction it's set to.
If the direction changed from the last direction seen for *this caret*,
we fired a SelectionLanguageChange notification on the caret's current
Selection. This looked bogus because the caret can be switched between
selections so it would seem some selections won't get a notification when
they should, but that's how it was. Also, when the SelectionLanguageChange
notification fired we then didn't draw the caret in that iteration, which
seems even more bogus.
This patch fixes all that by moving the logic to fire SelectionLanguageChange
out to GetPaintGeometry and firing the notification every single time without
trying to detect whether the state has changed or not. I carefully examined
the implementation of SelectionLanguageChange and I'm pretty sure it's
idempotent so this should be correct. That doesn't look like an
expensive function, and runs at most once per window paint, so I'm not
worried about perf. Because we now fire SelectionLanguageChange before
reading selection or frame state, it should be fine to carry on after
calling SelectionLanguageChange and drawing the caret based on whatever
changes SelectionLanguageChange has performed.
This also lets us remove mKeyboardRTL, which as noted above seems inherently
bogus.
--HG--
extra : rebase_source : 3ddfd10f6f30033e090e72b4bb43f2695218752e
Also, moves the "If the offset falls outside of the frame" check from
PaintCaret to GetPaintGeometry so we do less work in that case.
UpdateCaretRects is no longer needed.
--HG--
extra : rebase_source : 4b2925952a34d0388ae44c642129ce9015c367ea
This duplicates some code, but later patches will modify the callers and then
eventually we'll re-share common code.
--HG--
extra : rebase_source : 53f4756e87aadf22046972ef9102c190fbb35132
This is the start of the changes to caret-drawing proper.
The idea is to combine GetCaretFrame and GetCaretRect into a method
GetPaintGeometry which looks like GetGeometry but returns values
needed for painting (i.e. including bidi decorations, and returning
a null frame if we're not supposed to paint due to specific caret
state, e.g. in the "off" phase of the blink cycle).
Mostly a straightforward refactoring but there are a few interesting changes:
-- nsDisplayCaret stores its bounds instead of getting them from nsCaret on
demand. Eventually those bounds will not be stored in nsCaret at all.
-- nsDisplayCaret::GetBounds returns true for aSnap. nsCaret draws snapped
rects, so why not.
-- I removed "if (caretRect.Intersects(aDirtyRect))" in EnterPresShell.
As far as I can tell, this check is incorrect because it doesn't take
transforms into account. Since there's at most one drawn caret per window,
hence we do this at most once per paint, I don't think there's any real
performance advantage to having this check.
--HG--
extra : rebase_source : c98d3a5994478b482d19cc2e2ac83ab51bd17e00
The forward declaration of Selection in nsCaret.h will be used in later patches.
--HG--
extra : rebase_source : d1b749adac983c04d3365bb6bfb76a50101beeb5
This patch started an attempt to remove nsFrameSelection.h from nsCaret.h
and metastasized into a rather large refactoring patch that removed it
from some other header files as well, and changed nsFrameSelection::HINT
into a global-scope enum with better names. I also converted bools
into CaretAssociationHint in a few places where that was appropriate,
but there are still some more places (GetChildFrameContainingOffset)
where bools need to be converted. I figured this patch was big enough already.
--HG--
extra : rebase_source : cc618ef60e707e1360644340a2648de389383da0
GetGeometry is used in two different ways. Sometimes it's used to get
information about a particular caret. Sometimes it's used to get
information about a particular selection that's not associated with
a caret. Splitting GetGeometry into a non-static version for the former
and a static version for the latter makes this more clear. Also it saves
code since for the latter version we don't have to get an nsCaret first.
--HG--
extra : rebase_source : b7730dac56b308a82b79b175749234c9a92b6f59
