author Emilio Cobos Álvarez <>
Mon, 26 Nov 2018 09:21:37 +0000
changeset 504393 8e88421b280c2afda62f4ba704ce29701c30549f
parent 504375 1575904619b592d61f38ce67ef94faa313108a2e
permissions -rw-r--r--
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): 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:

<!doctype html>
<html class="reftest-wait">
<title>Caret on editable line with non-editable content and whitespace.</title>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
 * { outline: none }

 div {
  border: 1px solid red;
  margin: 5px;
  padding: 2px;
<div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>
SimpleTest.waitForFocus(function() {
  const editable = document.querySelector('div[contenteditable="true"]');
  setTimeout(() => {
    setTimeout(() => document.documentElement.className = "");