Diff
Modified: trunk/LayoutTests/ChangeLog (181417 => 181418)
--- trunk/LayoutTests/ChangeLog 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/LayoutTests/ChangeLog 2015-03-12 00:33:07 UTC (rev 181418)
@@ -1,3 +1,23 @@
+2015-03-11 Timothy Horton <[email protected]>
+
+ <attachment> shouldn't use "user-select: all"
+ https://bugs.webkit.org/show_bug.cgi?id=142453
+
+ Reviewed by Darin Adler.
+
+ * fast/attachment/attachment-select-on-click-inside-user-select-all.html: Added.
+ * fast/attachment/attachment-select-on-click.html: Added.
+ * platform/mac/fast/attachment/attachment-select-on-click-expected.png: Added.
+ * platform/mac/fast/attachment/attachment-select-on-click-expected.txt: Added.
+ * platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.png: Added.
+ * platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt: Added.
+ * platform/mac-mavericks/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt: Added.
+ * platform/mac-mavericks/fast/attachment/attachment-select-on-click-expected.txt: Added.
+ Add two tests. One, for the basic functionality of clicking on an
+ <attachment> to select it. The second, to test that clicking on an
+ <attachment> inside a larger "user-select: all" element still selects
+ the whole "user-select: all" element.
+
2015-03-11 Matthew Mirman <[email protected]>
Update windows test results
Added: trunk/LayoutTests/fast/attachment/attachment-select-on-click-inside-user-select-all.html (0 => 181418)
--- trunk/LayoutTests/fast/attachment/attachment-select-on-click-inside-user-select-all.html (rev 0)
+++ trunk/LayoutTests/fast/attachment/attachment-select-on-click-inside-user-select-all.html 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body _onload_="runTest()">
+<div style="-webkit-user-select: all;">text before <attachment id="attachment"></attachment> text after</div>
+<script>
+var file;
+if (window.internals)
+ file = window.internals.createFile("resources/test-file.txt");
+
+var attachment = document.getElementById('attachment');
+attachment.file = file;
+
+window._onload_ = function () {
+ if (!window.eventSender)
+ return;
+ eventSender.mouseMoveTo(attachment.offsetLeft + 10, attachment.offsetTop + 10);
+ eventSender.mouseDown(0);
+ eventSender.mouseUp(0);
+}
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/attachment/attachment-select-on-click.html (0 => 181418)
--- trunk/LayoutTests/fast/attachment/attachment-select-on-click.html (rev 0)
+++ trunk/LayoutTests/fast/attachment/attachment-select-on-click.html 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body _onload_="runTest()">
+<div>text before <attachment id="attachment"></attachment> text after</div>
+<script>
+var file;
+if (window.internals)
+ file = window.internals.createFile("resources/test-file.txt");
+
+var attachment = document.getElementById('attachment');
+attachment.file = file;
+
+window._onload_ = function () {
+ if (!window.eventSender)
+ return;
+ eventSender.mouseMoveTo(attachment.offsetLeft + 10, attachment.offsetTop + 10);
+ eventSender.mouseDown(0);
+ eventSender.mouseUp(0);
+}
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-expected.png
(Binary files differ)
Added: svn:mime-type
Added: trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-expected.txt (0 => 181418)
--- trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-expected.txt 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,13 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x93
+ RenderBlock {HTML} at (0,0) size 800x93
+ RenderBody {BODY} at (8,8) size 784x77
+ RenderBlock {DIV} at (0,0) size 784x77
+ RenderText {#text} at (0,54) size 73x18
+ text run at (0,54) width 73: "text before "
+ RenderAttachment {ATTACHMENT} at (72,0) size 75x77
+ RenderText {#text} at (146,54) size 63x18
+ text run at (146,54) width 63: " text after"
+selection start: position 0 of child 1 {ATTACHMENT} of child 1 {DIV} of body
+selection end: position 1 of child 1 {ATTACHMENT} of child 1 {DIV} of body
Added: trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.png
(Binary files differ)
Added: svn:mime-type
Added: trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt (0 => 181418)
--- trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,13 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x93
+ RenderBlock {HTML} at (0,0) size 800x93
+ RenderBody {BODY} at (8,8) size 784x77
+ RenderBlock {DIV} at (0,0) size 784x77
+ RenderText {#text} at (0,54) size 73x18
+ text run at (0,54) width 73: "text before "
+ RenderAttachment {ATTACHMENT} at (72,0) size 75x77
+ RenderText {#text} at (146,54) size 63x18
+ text run at (146,54) width 63: " text after"
+selection start: position 0 of child 0 {#text} of child 1 {DIV} of body
+selection end: position 11 of child 2 {#text} of child 1 {DIV} of body
Added: trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-expected.txt (0 => 181418)
--- trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-expected.txt 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,13 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x93
+ RenderBlock {HTML} at (0,0) size 800x93
+ RenderBody {BODY} at (8,8) size 784x77
+ RenderBlock {DIV} at (0,0) size 784x77
+ RenderText {#text} at (0,54) size 73x18
+ text run at (0,54) width 73: "text before "
+ RenderAttachment {ATTACHMENT} at (72,0) size 79x77
+ RenderText {#text} at (150,54) size 63x18
+ text run at (150,54) width 63: " text after"
+selection start: position 0 of child 1 {ATTACHMENT} of child 1 {DIV} of body
+selection end: position 1 of child 1 {ATTACHMENT} of child 1 {DIV} of body
Added: trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt (0 => 181418)
--- trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac-mavericks/fast/attachment/attachment-select-on-click-inside-user-select-all-expected.txt 2015-03-12 00:33:07 UTC (rev 181418)
@@ -0,0 +1,13 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x93
+ RenderBlock {HTML} at (0,0) size 800x93
+ RenderBody {BODY} at (8,8) size 784x77
+ RenderBlock {DIV} at (0,0) size 784x77
+ RenderText {#text} at (0,54) size 73x18
+ text run at (0,54) width 73: "text before "
+ RenderAttachment {ATTACHMENT} at (72,0) size 79x77
+ RenderText {#text} at (150,54) size 63x18
+ text run at (150,54) width 63: " text after"
+selection start: position 0 of child 0 {#text} of child 1 {DIV} of body
+selection end: position 11 of child 2 {#text} of child 1 {DIV} of body
Modified: trunk/Source/WebCore/ChangeLog (181417 => 181418)
--- trunk/Source/WebCore/ChangeLog 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/Source/WebCore/ChangeLog 2015-03-12 00:33:07 UTC (rev 181418)
@@ -1,3 +1,52 @@
+2015-03-11 Timothy Horton <[email protected]>
+
+ <attachment> shouldn't use "user-select: all"
+ https://bugs.webkit.org/show_bug.cgi?id=142453
+
+ Reviewed by Darin Adler.
+
+ It turns out that "user-select: all" is rife with bugs; in lieu of fixing them
+ all (at least for now), let's not use "user-select: all" in the default stylesheet
+ for <attachment>. It's really overkill anyway, since <attachment> can't have children.
+ The only "user-select: all" behavior we actually want is select-on-click.
+ So, we'll implement that in a slightly different way.
+
+ Tests: fast/attachment/attachment-select-on-click-inside-user-select-all.html
+ fast/attachment/attachment-select-on-click.html
+
+ * css/html.css:
+ (attachment):
+ No more "user-select: all".
+
+ (attachment:focus): Deleted.
+ We stopped using attachment focus a while back and forgot to remove this.
+
+ * dom/Node.h:
+ (WebCore::Node::shouldSelectOnMouseDown):
+ Add a virtual function that Node subclasses can override to indicate they
+ should be selected on mouse down.
+
+ * html/HTMLAttachmentElement.h:
+ Override the aforementioned virtual function; <attachment> should always
+ be selected on mouse down.
+
+ * page/EventHandler.cpp:
+ (WebCore::nodeToSelectOnMouseDownForNode):
+ Determine which node should be selected when a mousedown hits the given node.
+ If there's any "user-select: all", we go with the outermost "user-select: all".
+ Otherwise, we give the node a chance to say that it wants to be selected itself.
+
+ (WebCore::expandSelectionToRespectSelectOnMouseDown):
+ Rename this function, it's not just about "user-select: all" anymore.
+ Make use of nodeToSelectOnMouseDownForNode.
+
+ (WebCore::EventHandler::selectClosestWordFromHitTestResult):
+ (WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent):
+ (WebCore::EventHandler::handleMousePressEventTripleClick):
+ (WebCore::EventHandler::handleMousePressEventSingleClick):
+ (WebCore::expandSelectionToRespectUserSelectAll): Deleted.
+ Adjust to the new names.
+
2015-03-11 Geoffrey Garen <[email protected]>
Users of Heap::deprecatedReportExtraMemory should switch to reportExtraMemoryAllocated+reportExtraMemoryVisited
Modified: trunk/Source/WebCore/css/html.css (181417 => 181418)
--- trunk/Source/WebCore/css/html.css 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/Source/WebCore/css/html.css 2015-03-12 00:33:07 UTC (rev 181418)
@@ -1211,12 +1211,7 @@
#if defined(ENABLE_ATTACHMENT_ELEMENT) && ENABLE_ATTACHMENT_ELEMENT
attachment {
-webkit-appearance: attachment;
- -webkit-user-select: all;
}
-
-attachment:focus {
- outline: none;
-}
#endif
/* page */
Modified: trunk/Source/WebCore/dom/Node.h (181417 => 181418)
--- trunk/Source/WebCore/dom/Node.h 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/Source/WebCore/dom/Node.h 2015-03-12 00:33:07 UTC (rev 181418)
@@ -431,6 +431,8 @@
// Whether or not a selection can be started in this object
virtual bool canStartSelection() const;
+ virtual bool shouldSelectOnMouseDown() { return false; }
+
// Getting points into and out of screen space
FloatPoint convertToPage(const FloatPoint&) const;
FloatPoint convertFromPage(const FloatPoint&) const;
Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.h (181417 => 181418)
--- trunk/Source/WebCore/html/HTMLAttachmentElement.h 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.h 2015-03-12 00:33:07 UTC (rev 181418)
@@ -48,6 +48,7 @@
virtual RenderPtr<RenderElement> createElementRenderer(Ref<RenderStyle>&&) override;
+ virtual bool shouldSelectOnMouseDown() override { return true; }
virtual bool canContainRangeEndPoint() const override { return false; }
virtual void parseAttribute(const QualifiedName&, const AtomicString&) override;
Modified: trunk/Source/WebCore/page/EventHandler.cpp (181417 => 181418)
--- trunk/Source/WebCore/page/EventHandler.cpp 2015-03-12 00:29:45 UTC (rev 181417)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2015-03-12 00:33:07 UTC (rev 181418)
@@ -522,22 +522,30 @@
return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
}
-static VisibleSelection expandSelectionToRespectUserSelectAll(Node* targetNode, const VisibleSelection& selection)
+static Node* nodeToSelectOnMouseDownForNode(Node& targetNode)
{
#if ENABLE(USERSELECT_ALL)
- Node* rootUserSelectAll = Position::rootUserSelectAllForNode(targetNode);
- if (!rootUserSelectAll)
+ if (Node* rootUserSelectAll = Position::rootUserSelectAllForNode(&targetNode))
+ return rootUserSelectAll;
+#endif
+
+ if (targetNode.shouldSelectOnMouseDown())
+ return &targetNode;
+
+ return nullptr;
+}
+
+static VisibleSelection expandSelectionToRespectSelectOnMouseDown(Node& targetNode, const VisibleSelection& selection)
+{
+ Node* nodeToSelect = nodeToSelectOnMouseDownForNode(targetNode);
+ if (!nodeToSelect)
return selection;
VisibleSelection newSelection(selection);
- newSelection.setBase(positionBeforeNode(rootUserSelectAll).upstream(CanCrossEditingBoundary));
- newSelection.setExtent(positionAfterNode(rootUserSelectAll).downstream(CanCrossEditingBoundary));
+ newSelection.setBase(positionBeforeNode(nodeToSelect).upstream(CanCrossEditingBoundary));
+ newSelection.setExtent(positionAfterNode(nodeToSelect).downstream(CanCrossEditingBoundary));
return newSelection;
-#else
- UNUSED_PARAM(targetNode);
- return selection;
-#endif
}
bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& selection, TextGranularity granularity)
@@ -575,7 +583,7 @@
if (appendTrailingWhitespace == ShouldAppendTrailingWhitespace && newSelection.isRange())
newSelection.appendTrailingWhitespace();
- updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), WordGranularity);
+ updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectSelectOnMouseDown(targetNode, newSelection), WordGranularity);
}
}
@@ -601,7 +609,7 @@
if (pos.isNotNull() && pos.deepEquivalent().deprecatedNode()->isDescendantOf(URLElement))
newSelection = VisibleSelection::selectionFromContentsOfNode(URLElement);
- updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), WordGranularity);
+ updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectSelectOnMouseDown(targetNode, newSelection), WordGranularity);
}
}
@@ -639,7 +647,7 @@
newSelection.expandUsingGranularity(ParagraphGranularity);
}
- return updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), ParagraphGranularity);
+ return updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectSelectOnMouseDown(targetNode, newSelection), ParagraphGranularity);
}
static int textDistance(const Position& start, const Position& end)
@@ -677,7 +685,7 @@
TextGranularity granularity = CharacterGranularity;
if (extendSelection && newSelection.isCaretOrRange()) {
- VisibleSelection selectionInUserSelectAll = expandSelectionToRespectUserSelectAll(targetNode, VisibleSelection(pos));
+ VisibleSelection selectionInUserSelectAll = expandSelectionToRespectSelectOnMouseDown(targetNode, VisibleSelection(pos));
if (selectionInUserSelectAll.isRange()) {
if (comparePositions(selectionInUserSelectAll.start(), newSelection.start()) < 0)
pos = selectionInUserSelectAll.start();
@@ -704,7 +712,7 @@
newSelection.expandUsingGranularity(m_frame.selection().granularity());
}
} else
- newSelection = expandSelectionToRespectUserSelectAll(targetNode, visiblePos);
+ newSelection = expandSelectionToRespectSelectOnMouseDown(targetNode, visiblePos);
bool handled = updateSelectionForMouseDownDispatchingSelectStart(targetNode, newSelection, granularity);
