- Revision
- 136642
- Author
- [email protected]
- Date
- 2012-12-04 22:23:16 -0800 (Tue, 04 Dec 2012)
Log Message
Source/WebCore: Clicking a scrollbar unfocuses the current activeElement
https://bugs.webkit.org/show_bug.cgi?id=96335
Patch by Elliott Sprehn <[email protected]> on 2012-12-04
Reviewed by Ojan Vafai.
Previously we only tested for clicks inside frame scrollbars before
moving the focus, this patch expands the check to overflow scrollbars.
Now clicking inside a scrollbar only moves the focus when the scrollbar
has an ancestor that is mouse focusable.
This matches Gecko behavior, and was agreed to be the most sensible for now:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-October/037759.html
Test: fast/overflow/scrollbar-click-retains-focus.html
* page/EventHandler.cpp:
(WebCore::EventHandler::handleMousePressEvent):
Never start selections inside scrollbars because it would cause asserts.
This wasn't a problem before because we always moved the focus when clicking a scrollbar.
(WebCore::EventHandler::dispatchMouseEvent):
Check that the click is not inside a scrollbar before moving the focus.
(WebCore::EventHandler::isInsideScrollbar): Tests if a point is in a scrollbar.
(WebCore):
(WebCore::EventHandler::sendContextMenuEvent):
Never start selections inside scrollbars because it would cause asserts.
* page/EventHandler.h:
(EventHandler):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::hitTestOverflowControls):
LayoutTests: Clicking a scrollbar unfocuses the current activeElement
https://bugs.webkit.org/show_bug.cgi?id=96335
Patch by Elliott Sprehn <[email protected]> on 2012-12-04
Reviewed by Ojan Vafai.
Add test that ensures clicking inside a scrollbar doesn't move the
focus unless the scrollbar is inside a mouse focusable element.
* fast/overflow/scrollbar-click-retains-focus-expected.txt: Added.
* fast/overflow/scrollbar-click-retains-focus.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136641 => 136642)
--- trunk/LayoutTests/ChangeLog 2012-12-05 06:20:57 UTC (rev 136641)
+++ trunk/LayoutTests/ChangeLog 2012-12-05 06:23:16 UTC (rev 136642)
@@ -1,3 +1,16 @@
+2012-12-04 Elliott Sprehn <[email protected]>
+
+ Clicking a scrollbar unfocuses the current activeElement
+ https://bugs.webkit.org/show_bug.cgi?id=96335
+
+ Reviewed by Ojan Vafai.
+
+ Add test that ensures clicking inside a scrollbar doesn't move the
+ focus unless the scrollbar is inside a mouse focusable element.
+
+ * fast/overflow/scrollbar-click-retains-focus-expected.txt: Added.
+ * fast/overflow/scrollbar-click-retains-focus.html: Added.
+
2012-12-04 Viatcheslav Ostapenko <[email protected]>
[EFL][WK2] Enable compositing pixel tests that are not fail after EFL WTR snapshot implementation.
Added: trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus-expected.txt (0 => 136642)
--- trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus-expected.txt 2012-12-05 06:23:16 UTC (rev 136642)
@@ -0,0 +1,28 @@
+This tests clicking scrollbars, which should only move the focus if an ancestor is mouse focusable.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+Focus should remain in the input
+PASS document.activeElement.tagName is "INPUT"
+
+Focus should move if ancestor is mouse focusable
+PASS document.activeElement.tagName is "DIV"
+PASS document.activeElement.tagName is "DIV"
+
+Focus should move if ancestor is content editable
+PASS document.activeElement.tagName is "SPAN"
+PASS document.activeElement.tagName is "SPAN"
+
+Form controls should move the focus
+PASS document.activeElement.tagName is "TEXTAREA"
+PASS document.activeElement.tagName is "SELECT"
+
+Disabled form controls should not move the focus
+PASS document.activeElement.tagName is "INPUT"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
Added: trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html (0 => 136642)
--- trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html 2012-12-05 06:23:16 UTC (rev 136642)
@@ -0,0 +1,120 @@
+<!DOCTYPE html>
+
+<script src=""
+
+<style>
+ /* Float everything so they fit inside the viewport for using eventSender to click */
+ section, footer, span, textarea, select { float: left; }
+</style>
+
+<footer>
+ <input>
+</footer>
+
+<section style="height: 100px; width: 100px; overflow: scroll;"></section>
+
+<div tabindex="1">
+ <header style="height: 100px; width: 100px; overflow: scroll;"></header>
+</div>
+
+<span contenteditable>
+ <u style="height: 100px; width: 100px; overflow: scroll; display: block;"></u>
+</span>
+
+<textarea rows="5">
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+ Text.
+</textarea>
+
+<select multiple>
+ <option> Text.
+ <option> Text.
+ <option> Text.
+ <option> Text.
+ <option> Text.
+ <option> Text.
+ <option> Text.
+</select>
+
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+function click(x, y)
+{
+ if (window.eventSender) {
+ eventSender.mouseMoveTo(x, y);
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+ }
+}
+
+function clickVerticalScrollbar(type)
+{
+ var element = document.querySelector(type);
+ click(element.offsetLeft + element.offsetWidth - 5, element.offsetTop + 5);
+}
+
+function clickHorizontalScrollbar(type)
+{
+ var element = document.querySelector(type);
+ click(element.offsetLeft + 5, element.offsetTop + element.offsetHeight - 5);
+}
+
+function test(name, fn)
+{
+ debug("<br>" + name);
+ fn();
+}
+
+description("This tests clicking scrollbars, which should only move the focus if an ancestor is mouse focusable.");
+
+test("Focus should remain in the input", function() {
+ document.querySelector("input").focus();
+ clickVerticalScrollbar("section");
+ clickHorizontalScrollbar("section");
+ shouldBeEqualToString("document.activeElement.tagName", "INPUT");
+});
+
+test("Focus should move if ancestor is mouse focusable", function() {
+ document.querySelector("input").focus();
+ clickVerticalScrollbar("header");
+ shouldBeEqualToString("document.activeElement.tagName", "DIV");
+ document.querySelector("input").focus();
+ clickHorizontalScrollbar("header");
+ shouldBeEqualToString("document.activeElement.tagName", "DIV");
+});
+
+test("Focus should move if ancestor is content editable", function() {
+ document.querySelector("input").focus();
+ clickVerticalScrollbar("u");
+ shouldBeEqualToString("document.activeElement.tagName", "SPAN");
+ document.querySelector("input").focus();
+ clickHorizontalScrollbar("u");
+ shouldBeEqualToString("document.activeElement.tagName", "SPAN");
+});
+
+test("Form controls should move the focus", function() {
+ clickVerticalScrollbar("textarea");
+ shouldBeEqualToString("document.activeElement.tagName", "TEXTAREA");
+ clickVerticalScrollbar("select");
+ shouldBeEqualToString("document.activeElement.tagName", "SELECT");
+});
+
+test("Disabled form controls should not move the focus", function() {
+ document.querySelector("input").focus();
+ document.querySelector("select").disabled = true;
+ clickVerticalScrollbar("select");
+ shouldBeEqualToString("document.activeElement.tagName", "INPUT");
+});
+</script>
+
+<script src=""
Modified: trunk/Source/WebCore/ChangeLog (136641 => 136642)
--- trunk/Source/WebCore/ChangeLog 2012-12-05 06:20:57 UTC (rev 136641)
+++ trunk/Source/WebCore/ChangeLog 2012-12-05 06:23:16 UTC (rev 136642)
@@ -1,3 +1,35 @@
+2012-12-04 Elliott Sprehn <[email protected]>
+
+ Clicking a scrollbar unfocuses the current activeElement
+ https://bugs.webkit.org/show_bug.cgi?id=96335
+
+ Reviewed by Ojan Vafai.
+
+ Previously we only tested for clicks inside frame scrollbars before
+ moving the focus, this patch expands the check to overflow scrollbars.
+ Now clicking inside a scrollbar only moves the focus when the scrollbar
+ has an ancestor that is mouse focusable.
+
+ This matches Gecko behavior, and was agreed to be the most sensible for now:
+ http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-October/037759.html
+
+ Test: fast/overflow/scrollbar-click-retains-focus.html
+
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::handleMousePressEvent):
+ Never start selections inside scrollbars because it would cause asserts.
+ This wasn't a problem before because we always moved the focus when clicking a scrollbar.
+ (WebCore::EventHandler::dispatchMouseEvent):
+ Check that the click is not inside a scrollbar before moving the focus.
+ (WebCore::EventHandler::isInsideScrollbar): Tests if a point is in a scrollbar.
+ (WebCore):
+ (WebCore::EventHandler::sendContextMenuEvent):
+ Never start selections inside scrollbars because it would cause asserts.
+ * page/EventHandler.h:
+ (EventHandler):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::hitTestOverflowControls):
+
2012-12-04 Kentaro Hara <[email protected]>
[V8] Replace String::New("symbol") with String::NewSymbol("symbol")
Modified: trunk/Source/WebCore/page/EventHandler.cpp (136641 => 136642)
--- trunk/Source/WebCore/page/EventHandler.cpp 2012-12-05 06:20:57 UTC (rev 136641)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2012-12-05 06:23:16 UTC (rev 136642)
@@ -645,8 +645,8 @@
bool singleClick = event.event().clickCount() <= 1;
// If we got the event back, that must mean it wasn't prevented,
- // so it's allowed to start a drag or selection.
- m_mouseDownMayStartSelect = canMouseDownStartSelect(event.targetNode());
+ // so it's allowed to start a drag or selection if it wasn't in a scrollbar.
+ m_mouseDownMayStartSelect = canMouseDownStartSelect(event.targetNode()) && !event.scrollbar();
#if ENABLE(DRAG_SUPPORT)
// Careful that the drag starting logic stays in sync with eventMayStartDrag()
@@ -2395,6 +2395,10 @@
node = node->parentOrHostNode();
}
+ // Only change the focus when clicking scrollbars if it can transfered to a mouse focusable node.
+ if ((!node || !node->isMouseFocusable()) && isInsideScrollbar(mouseEvent.position()))
+ return false;
+
// If focus shift is blocked, we eat the event. Note we should never clear swallowEvent
// if the page already set it (e.g., by canceling default behavior).
if (Page* page = m_frame->page()) {
@@ -2411,6 +2415,18 @@
return !swallowEvent;
}
+bool EventHandler::isInsideScrollbar(const IntPoint& windowPoint) const
+{
+ if (RenderView* renderView = m_frame->contentRenderer()) {
+ HitTestRequest request(HitTestRequest::ReadOnly);
+ HitTestResult result(windowPoint);
+ renderView->hitTest(request, result);
+ return result.scrollbar();
+ }
+
+ return false;
+}
+
#if !PLATFORM(GTK) && !(PLATFORM(CHROMIUM) && (OS(UNIX) && !OS(DARWIN)))
bool EventHandler::shouldTurnVerticalTicksIntoHorizontal(const HitTestResult&, const PlatformWheelEvent&) const
{
@@ -2834,6 +2850,7 @@
if (m_frame->editor()->behavior().shouldSelectOnContextualMenuClick()
&& !m_frame->selection()->contains(viewportPos)
+ && !mev.scrollbar()
// FIXME: In the editable case, word selection sometimes selects content that isn't underneath the mouse.
// If the selection is non-editable, we do word selection to make it easier to use the contextual menu items
// available for text selections. But only if we're above text.
Modified: trunk/Source/WebCore/page/EventHandler.h (136641 => 136642)
--- trunk/Source/WebCore/page/EventHandler.h 2012-12-05 06:20:57 UTC (rev 136641)
+++ trunk/Source/WebCore/page/EventHandler.h 2012-12-05 06:23:16 UTC (rev 136642)
@@ -290,6 +290,8 @@
void fakeMouseMoveEventTimerFired(DeferrableOneShotTimer<EventHandler>*);
void cancelFakeMouseMoveEvent();
+ bool isInsideScrollbar(const IntPoint&) const;
+
#if ENABLE(TOUCH_EVENTS)
bool dispatchSyntheticTouchEventIfEnabled(const PlatformMouseEvent&);
#endif
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (136641 => 136642)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-12-05 06:20:57 UTC (rev 136641)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-12-05 06:23:16 UTC (rev 136642)
@@ -2973,6 +2973,8 @@
int resizeControlSize = max(resizeControlRect.height(), 0);
+ // FIXME: We should hit test the m_scrollCorner and pass it back through the result.
+
if (m_vBar && m_vBar->shouldParticipateInHitTesting()) {
LayoutRect vBarRect(verticalScrollbarStart(0, box->width()),
box->borderTop(),
