Diff
Modified: trunk/LayoutTests/ChangeLog (284365 => 284366)
--- trunk/LayoutTests/ChangeLog 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/LayoutTests/ChangeLog 2021-10-18 12:26:37 UTC (rev 284366)
@@ -1,3 +1,23 @@
+2021-10-18 Tim Nguyen <[email protected]>
+
+ Fix mouse selection on modal <dialog> text nodes
+ https://bugs.webkit.org/show_bug.cgi?id=231741
+
+ Reviewed by Antti Koivisto.
+
+ Tests:
+ - editing/selection/modal-dialog-select-paragraph.html (tests this bug)
+ - imported/w3c/web-platform-tests/inert/inert-node-is-unselectable.tentative.html (tests what
+ the canStartSelection check was supposed to fix)
+
+ The inert check in Node::canStartSelection() was too restrictive, which caused this bug to happen because
+ we set the root node as inert and then reset it for the modal <dialog> itself, causing the text in the
+ <dialog> to be unselectable. Instead, just mirror how -webkit-user-select: none; is implemented by
+ introducing RenderStyle::userSelectIncludingInert().
+
+ * editing/selection/modal-dialog-select-paragraph-expected.txt: Added.
+ * editing/selection/modal-dialog-select-paragraph.html: Added.
+
2021-10-18 Enrique Ocaña González <[email protected]>
[GLIB][GStreamer] test http/tests/security/webaudio-render-remote-audio-allowed-crossorigin-redirect.html is a flaky timeout since r278004
Added: trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph-expected.txt (0 => 284366)
--- trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph-expected.txt 2021-10-18 12:26:37 UTC (rev 284366)
@@ -0,0 +1,3 @@
+This text should be selectable with the mouse.
+
+PASS: all dialog text is selected
Added: trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph.html (0 => 284366)
--- trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph.html (rev 0)
+++ trunk/LayoutTests/editing/selection/modal-dialog-select-paragraph.html 2021-10-18 12:26:37 UTC (rev 284366)
@@ -0,0 +1,30 @@
+<dialog><p>This text should be selectable with the mouse.</p></dialog>
+<div id="log"></div>
+<script>
+document.querySelector("dialog").showModal();
+
+function selectParagraphWithMouse(p) {
+ const rect = p.getBoundingClientRect();
+ const x = rect.left;
+ const y = rect.top + rect.height / 2;
+ const textLength = p.textContent.length;
+ eventSender.mouseMoveTo(x, y);
+ eventSender.mouseDown();
+ eventSender.leapForward(200);
+ eventSender.mouseMoveTo(x + rect.width, y);
+ eventSender.mouseUp();
+}
+
+if (window.eventSender) {
+ testRunner.dumpAsText();
+
+ const p = document.querySelector("p");
+ selectParagraphWithMouse(p);
+
+ const selection = getSelection().toString();
+ if (selection == p.textContent)
+ log.textContent = "PASS: all dialog text is selected";
+ else
+ log.textContent = "FAIL: unexpected selection, got: " + selection;
+}
+</script>
Modified: trunk/Source/WebCore/ChangeLog (284365 => 284366)
--- trunk/Source/WebCore/ChangeLog 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/ChangeLog 2021-10-18 12:26:37 UTC (rev 284366)
@@ -1,3 +1,47 @@
+2021-10-18 Tim Nguyen <[email protected]>
+
+ Fix mouse selection on modal <dialog> text nodes
+ https://bugs.webkit.org/show_bug.cgi?id=231741
+
+ Reviewed by Antti Koivisto.
+
+ Tests:
+ - editing/selection/modal-dialog-select-paragraph.html (tests this bug)
+ - imported/w3c/web-platform-tests/inert/inert-node-is-unselectable.tentative.html (tests what
+ the canStartSelection check was supposed to fix)
+
+ The inert check in Node::canStartSelection() was too restrictive, which caused this bug to happen because
+ we set the root node as inert and then reset it for the modal <dialog> itself, causing the text in the
+ <dialog> to be unselectable. Instead, just mirror how -webkit-user-select: none; is implemented by
+ introducing RenderStyle::userSelectIncludingInert().
+
+ * dom/Node.cpp:
+ (WebCore::computeEditabilityFromComputedStyle):
+ (WebCore::Node::canStartSelection const):
+ * dom/Position.cpp:
+ (WebCore::Position::nodeIsUserSelectNone):
+ (WebCore::Position::nodeIsUserSelectAll):
+ (WebCore::Position::isCandidate const):
+ (WebCore::Position::nodeIsInertOrUserSelectNone): Deleted.
+ * dom/Position.h:
+ * dom/PositionIterator.cpp:
+ (WebCore::PositionIterator::isCandidate const):
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart):
+ (WebCore::EventHandler::canMouseDownStartSelect):
+ (WebCore::EventHandler::selectCursor):
+ * page/Frame.cpp:
+ (WebCore::Frame::rangeForPoint):
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::selectionColor const):
+ (WebCore::RenderElement::selectionBackgroundColor const):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::collectSelectionGeometriesInternal):
+ * rendering/style/RenderStyle.h:
+ (WebCore::RenderStyle::userSelectIncludingInert const):
+
2021-10-18 Enrique Ocaña González <[email protected]>
[GLIB][GStreamer] test http/tests/security/webaudio-render-remote-audio-allowed-crossorigin-redirect.html is a flaky timeout since r278004
Modified: trunk/Source/WebCore/dom/Node.cpp (284365 => 284366)
--- trunk/Source/WebCore/dom/Node.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/dom/Node.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -764,7 +764,7 @@
continue;
// Elements with user-select: all style are considered atomic
// therefore non editable.
- if (treatment == Node::UserSelectAllIsAlwaysNonEditable && style->userSelect() == UserSelect::All)
+ if (treatment == Node::UserSelectAllIsAlwaysNonEditable && style->userSelectIncludingInert() == UserSelect::All)
return Node::Editability::ReadOnly;
switch (style->userModify()) {
case UserModify::ReadOnly:
@@ -1130,12 +1130,10 @@
if (renderer()) {
const RenderStyle& style = renderer()->style();
- if (style.effectiveInert())
- return false;
// We allow selections to begin within an element that has -webkit-user-select: none set,
// but if the element is draggable then dragging should take priority over selection.
- if (style.userDrag() == UserDrag::Element && style.userSelect() == UserSelect::None)
+ if (style.userDrag() == UserDrag::Element && style.userSelectIncludingInert() == UserSelect::None)
return false;
}
return parentOrShadowHostNode() ? parentOrShadowHostNode()->canStartSelection() : true;
Modified: trunk/Source/WebCore/dom/Position.cpp (284365 => 284366)
--- trunk/Source/WebCore/dom/Position.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/dom/Position.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -937,11 +937,11 @@
return false;
}
-bool Position::nodeIsInertOrUserSelectNone(Node* node)
+bool Position::nodeIsUserSelectNone(Node* node)
{
if (!node)
return false;
- return node->renderer() && (node->renderer()->style().userSelect() == UserSelect::None || node->renderer()->style().effectiveInert());
+ return node->renderer() && (node->renderer()->style().userSelectIncludingInert() == UserSelect::None);
}
bool Position::nodeIsUserSelectAll(const Node* node)
@@ -948,7 +948,7 @@
{
if (!node)
return false;
- return node->renderer() && (node->renderer()->style().userSelect() == UserSelect::All && !node->renderer()->style().effectiveInert());
+ return node->renderer() && (node->renderer()->style().userSelectIncludingInert() == UserSelect::All);
}
Node* Position::rootUserSelectAllForNode(Node* node)
@@ -988,16 +988,16 @@
if (renderer->isBR()) {
// FIXME: The condition should be m_anchorType == PositionIsBeforeAnchor, but for now we still need to support legacy positions.
- return !m_offset && m_anchorType != PositionIsAfterAnchor && !nodeIsInertOrUserSelectNone(deprecatedNode()->parentNode());
+ return !m_offset && m_anchorType != PositionIsAfterAnchor && !nodeIsUserSelectNone(deprecatedNode()->parentNode());
}
if (is<RenderText>(*renderer))
- return !nodeIsInertOrUserSelectNone(deprecatedNode()) && downcast<RenderText>(*renderer).containsCaretOffset(m_offset);
+ return !nodeIsUserSelectNone(deprecatedNode()) && downcast<RenderText>(*renderer).containsCaretOffset(m_offset);
if (positionBeforeOrAfterNodeIsCandidate(*deprecatedNode())) {
return ((atFirstEditingPositionForNode() && m_anchorType == PositionIsBeforeAnchor)
|| (atLastEditingPositionForNode() && m_anchorType == PositionIsAfterAnchor))
- && !nodeIsInertOrUserSelectNone(deprecatedNode()->parentNode());
+ && !nodeIsUserSelectNone(deprecatedNode()->parentNode());
}
if (is<HTMLHtmlElement>(*m_anchorNode))
@@ -1007,13 +1007,13 @@
auto& block = downcast<RenderBlock>(*renderer);
if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode) || m_anchorNode->isRootEditableElement()) {
if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))
- return atFirstEditingPositionForNode() && !Position::nodeIsInertOrUserSelectNone(deprecatedNode());
- return m_anchorNode->hasEditableStyle() && !Position::nodeIsInertOrUserSelectNone(deprecatedNode()) && atEditingBoundary();
+ return atFirstEditingPositionForNode() && !Position::nodeIsUserSelectNone(deprecatedNode());
+ return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(deprecatedNode()) && atEditingBoundary();
}
return false;
}
- return m_anchorNode->hasEditableStyle() && !Position::nodeIsInertOrUserSelectNone(deprecatedNode()) && atEditingBoundary();
+ return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(deprecatedNode()) && atEditingBoundary();
}
bool Position::isRenderedCharacter() const
Modified: trunk/Source/WebCore/dom/Position.h (284365 => 284366)
--- trunk/Source/WebCore/dom/Position.h 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/dom/Position.h 2021-10-18 12:26:37 UTC (rev 284366)
@@ -179,7 +179,7 @@
static unsigned positionCountBetweenPositions(const Position&, const Position&);
static bool hasRenderedNonAnonymousDescendantsWithHeight(const RenderElement&);
- static bool nodeIsInertOrUserSelectNone(Node*);
+ static bool nodeIsUserSelectNone(Node*);
static bool nodeIsUserSelectAll(const Node*);
static Node* rootUserSelectAllForNode(Node*);
Modified: trunk/Source/WebCore/dom/PositionIterator.cpp (284365 => 284366)
--- trunk/Source/WebCore/dom/PositionIterator.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/dom/PositionIterator.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -162,10 +162,10 @@
return Position(*this).isCandidate();
if (is<RenderText>(*renderer))
- return !Position::nodeIsInertOrUserSelectNone(m_anchorNode) && downcast<RenderText>(*renderer).containsCaretOffset(m_offsetInAnchor);
+ return !Position::nodeIsUserSelectNone(m_anchorNode) && downcast<RenderText>(*renderer).containsCaretOffset(m_offsetInAnchor);
if (positionBeforeOrAfterNodeIsCandidate(*m_anchorNode))
- return (atStartOfNode() || atEndOfNode()) && !Position::nodeIsInertOrUserSelectNone(m_anchorNode->parentNode());
+ return (atStartOfNode() || atEndOfNode()) && !Position::nodeIsUserSelectNone(m_anchorNode->parentNode());
if (is<HTMLHtmlElement>(*m_anchorNode))
return false;
@@ -174,13 +174,13 @@
auto& block = downcast<RenderBlock>(*renderer);
if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode) || m_anchorNode->isRootEditableElement()) {
if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))
- return atStartOfNode() && !Position::nodeIsInertOrUserSelectNone(m_anchorNode);
- return m_anchorNode->hasEditableStyle() && !Position::nodeIsInertOrUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
+ return atStartOfNode() && !Position::nodeIsUserSelectNone(m_anchorNode);
+ return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
}
return false;
}
- return m_anchorNode->hasEditableStyle() && !Position::nodeIsInertOrUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
+ return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
}
} // namespace WebCore
Modified: trunk/Source/WebCore/page/EventHandler.cpp (284365 => 284366)
--- trunk/Source/WebCore/page/EventHandler.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -475,7 +475,7 @@
bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& selection, TextGranularity granularity)
{
- if (Position::nodeIsInertOrUserSelectNone(targetNode))
+ if (Position::nodeIsUserSelectNone(targetNode))
return false;
if (!dispatchSelectStart(targetNode)) {
@@ -715,12 +715,12 @@
if (!page->chrome().client().shouldUseMouseEventForSelection(event.event()))
return false;
}
-
+
if (!node || !node->renderer())
return true;
if (HTMLElement::isImageOverlayText(*node))
- return node->renderer()->style().userSelect() != UserSelect::None;
+ return node->renderer()->style().userSelectIncludingInert() != UserSelect::None;
return node->canStartSelection() || Position::nodeIsUserSelectAll(node.get());
}
@@ -1522,7 +1522,7 @@
case CursorType::Auto: {
if (HTMLElement::isImageOverlayText(node.get())) {
auto* renderer = node->renderer();
- if (renderer && renderer->style().userSelect() != UserSelect::None)
+ if (renderer && renderer->style().userSelectIncludingInert() != UserSelect::None)
return iBeam;
}
Modified: trunk/Source/WebCore/page/Frame.cpp (284365 => 284366)
--- trunk/Source/WebCore/page/Frame.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/page/Frame.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -863,7 +863,7 @@
auto position = visiblePositionForPoint(framePoint);
auto containerText = position.deepEquivalent().containerText();
- if (!containerText || !containerText->renderer() || containerText->renderer()->style().userSelect() == UserSelect::None)
+ if (!containerText || !containerText->renderer() || containerText->renderer()->style().userSelectIncludingInert() == UserSelect::None)
return std::nullopt;
if (auto previousCharacterRange = makeSimpleRange(position.previous(), position)) {
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (284365 => 284366)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -1543,7 +1543,7 @@
{
// If the element is unselectable, or we are only painting the selection,
// don't override the foreground color with the selection foreground color.
- if (style().userSelect() == UserSelect::None
+ if (style().userSelectIncludingInert() == UserSelect::None
|| (view().frameView().paintBehavior().containsAny({ PaintBehavior::SelectionOnly, PaintBehavior::SelectionAndBackgroundsOnly })))
return Color();
@@ -1598,7 +1598,7 @@
Color RenderElement::selectionBackgroundColor() const
{
- if (style().userSelect() == UserSelect::None)
+ if (style().userSelectIncludingInert() == UserSelect::None)
return Color();
if (frame().selection().shouldShowBlockCursor() && frame().selection().isCaret())
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (284365 => 284366)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -5248,7 +5248,7 @@
if (!renderText.hasRenderedText())
continue;
- if (renderer.style().userSelect() != UserSelect::None)
+ if (renderer.style().userSelectIncludingInert() != UserSelect::None)
request.setHasPaintedContent();
if (!renderText.text().isAllSpecialCharacters<isHTMLSpace>()) {
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (284365 => 284366)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -2181,7 +2181,7 @@
for (auto& node : intersectingNodesWithDeprecatedZeroOffsetStartQuirk(range)) {
auto renderer = node.renderer();
// Only ask leaf render objects for their line box rects.
- if (renderer && !renderer->firstChildSlow() && renderer->style().userSelect() != UserSelect::None && !renderer->style().effectiveInert()) {
+ if (renderer && !renderer->firstChildSlow() && renderer->style().userSelectIncludingInert() != UserSelect::None) {
bool isStartNode = renderer->node() == range.start.container.ptr();
bool isEndNode = renderer->node() == range.end.container.ptr();
if (hasFlippedWritingMode != renderer->style().isFlippedBlocksWritingMode())
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (284365 => 284366)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2021-10-18 12:26:37 UTC (rev 284366)
@@ -610,6 +610,7 @@
MarqueeDirection marqueeDirection() const { return static_cast<MarqueeDirection>(m_rareNonInheritedData->marquee->direction); }
UserModify userModify() const { return static_cast<UserModify>(m_rareInheritedData->userModify); }
UserDrag userDrag() const { return static_cast<UserDrag>(m_rareNonInheritedData->userDrag); }
+ UserSelect userSelectIncludingInert() const { return effectiveInert() ? UserSelect::None : userSelect(); }
UserSelect userSelect() const { return static_cast<UserSelect>(m_rareInheritedData->userSelect); }
TextOverflow textOverflow() const { return static_cast<TextOverflow>(m_rareNonInheritedData->textOverflow); }
MarginCollapse marginBeforeCollapse() const { return static_cast<MarginCollapse>(m_rareNonInheritedData->marginBeforeCollapse); }
Modified: trunk/Source/WebKit/ChangeLog (284365 => 284366)
--- trunk/Source/WebKit/ChangeLog 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebKit/ChangeLog 2021-10-18 12:26:37 UTC (rev 284366)
@@ -1,3 +1,25 @@
+2021-10-18 Tim Nguyen <[email protected]>
+
+ Fix mouse selection on modal <dialog> text nodes
+ https://bugs.webkit.org/show_bug.cgi?id=231741
+
+ Reviewed by Antti Koivisto.
+
+ Tests:
+ - editing/selection/modal-dialog-select-paragraph.html (tests this bug)
+ - imported/w3c/web-platform-tests/inert/inert-node-is-unselectable.tentative.html (tests what
+ the canStartSelection check was supposed to fix)
+
+ The inert check in Node::canStartSelection() was too restrictive, which caused this bug to happen because
+ we set the root node as inert and then reset it for the modal <dialog> itself, causing the text in the
+ <dialog> to be unselectable. Instead, just mirror how -webkit-user-select: none; is implemented by
+ introducing RenderStyle::userSelectIncludingInert().
+
+ * WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:
+ (WebKit::InjectedBundleNodeHandle::isSelectableTextNode const):
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::selectionPositionInformation):
+
2021-10-18 David Kilzer <[email protected]>
Use-after-move of m_sockets in NetworkRTCProvider::close()
Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp (284365 => 284366)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp 2021-10-18 12:26:37 UTC (rev 284366)
@@ -382,7 +382,7 @@
return false;
auto renderer = m_node->renderer();
- return renderer && renderer->style().userSelect() != UserSelect::None;
+ return renderer && renderer->style().userSelectIncludingInert() != UserSelect::None;
}
RefPtr<InjectedBundleNodeHandle> InjectedBundleNodeHandle::htmlTableCellElementCellAbove()
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (284365 => 284366)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-10-18 11:33:38 UTC (rev 284365)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-10-18 12:26:37 UTC (rev 284366)
@@ -2906,7 +2906,7 @@
if (attachment.file())
info.url = ""
} else {
- info.isSelectable = renderer->style().userSelect() != UserSelect::None && !renderer->style().effectiveInert();
+ info.isSelectable = renderer->style().userSelectIncludingInert() != UserSelect::None;
// We don't want to select blocks that are larger than 97% of the visible area of the document.
// FIXME: Is this heuristic still needed, now that block selection has been removed?
if (info.isSelectable && !hitNode->isTextNode())
@@ -2918,7 +2918,7 @@
continue;
auto& style = renderer->style();
- if (style.userSelect() == UserSelect::None && style.userDrag() == UserDrag::Element) {
+ if (style.userSelectIncludingInert() == UserSelect::None && style.userDrag() == UserDrag::Element) {
info.prefersDraggingOverTextSelection = true;
break;
}
