Diff
Modified: trunk/LayoutTests/ChangeLog (142955 => 142956)
--- trunk/LayoutTests/ChangeLog 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/ChangeLog 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,3 +1,13 @@
+2013-02-14 Simon Fraser <simon.fra...@apple.com>
+
+ Reverting r142861. Hit testing inside of style recalc is fundamentally wrong
+
+ * fast/events/mouse-cursor-change-expected.txt: Removed.
+ * fast/events/mouse-cursor-change.html: Removed.
+ * fast/events/mouse-cursor-no-mousemove-expected.txt: Removed.
+ * fast/events/mouse-cursor-no-mousemove.html: Removed.
+ * platform/mac/TestExpectations:
+
2013-02-14 Florin Malita <fmal...@chromium.org>
[SVG] Cached filter results are not invalidated on repaint rect change
@@ -352,25 +362,6 @@
* fast/text-autosizing/narrow-descendants-combined-expected.html: Added.
* fast/text-autosizing/narrow-descendants-combined.html: Added.
-2013-02-14 Aivo Paas <aivop...@gmail.com>
-
- Updating mouse cursor on style changes without emitting fake mousemove event
- https://bugs.webkit.org/show_bug.cgi?id=101857
- Changing CSS cursor should work no matter is mouse button is pressed or not
- https://bugs.webkit.org/show_bug.cgi?id=53341
-
- Reviewed by Allan Sandfeld Jensen.
-
- Added tests for changing cursor on mousemove, mousedown, mouseup and mousemove
- while mouse button being hold down. Also added test to verify that a mousemove
- event is not fired for changing cursor while mouse is not moving.
-
- * fast/events/mouse-cursor-change-expected.txt: Added.
- * fast/events/mouse-cursor-change.html: Added.
- * fast/events/mouse-cursor-no-mousemove-expected.txt: Added.
- * fast/events/mouse-cursor-no-mousemove.html: Added.
- * platform/mac/TestExpectations:
-
2013-02-06 Gregg Tavares <g...@chromium.org>
Adds the WebGL Conformance Tests limits folder.
Deleted: trunk/LayoutTests/fast/events/mouse-cursor-change-expected.txt (142955 => 142956)
--- trunk/LayoutTests/fast/events/mouse-cursor-change-expected.txt 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/fast/events/mouse-cursor-change-expected.txt 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,24 +0,0 @@
-Test that mouse cursors are changed correctly on mouse events.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-Bug 53341
-
-
-Mouse move
-Cursor Info: type=Hand hotSpot=0,0
-
-Mouse down
-Cursor Info: type=Progress hotSpot=0,0
-
-Mouse hold down, move
-Cursor Info: type=Hand hotSpot=0,0
-
-Mouse up
-Cursor Info: type=Help hotSpot=0,0
-
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/fast/events/mouse-cursor-change.html (142955 => 142956)
--- trunk/LayoutTests/fast/events/mouse-cursor-change.html 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/fast/events/mouse-cursor-change.html 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,78 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script src=""
-<style type="text/css">
-</style>
-</head>
-<body>
-<p id="description"></p>
-<p><a href="" 53341</a></p>
-<div id="test-container">
- <div id="target" _onMouseDown_="style.cursor='progress';event.preventDefault();" _onMouseMove_="style.cursor='pointer';" _onMouseUp_="style.cursor='help';" style="cursor:pointer;">Play with mouse on this element. Cursors change on events - mousemove: pointer(hand), mousedown: progress, mouseup: help.</div>
-</div>
-<br/>
-<div id="console"></div>
-<script>
-description("Test that mouse cursors are changed correctly on mouse events.");
-
-if (!window.eventSender) {
- testFailed('This test requires DumpRenderTree');
-}
-
-if (window.testRunner) {
- testRunner.dumpAsText();
- testRunner.waitUntilDone();
- window.jsTestIsAsync = true;
-}
-
-function runTest(prepare, next) {
- prepare();
- setTimeout(function() {
- debug('Cursor Info: ' + window.internals.getCurrentCursorInfo(document));
- debug('');
- next();
- }, 0);
-}
-
-function testsDone() {
- // This text is redundant with the test output - hide it
- document.getElementById('test-container').style.display = 'none';
- finishJSTest();
-}
-
-// Can't do anything useful here without eventSender
-if (window.eventSender) {
- var target = document.getElementById('target');
- eventSender.dragMode = false;
- var tests = [
- function() {
- debug('Mouse move');
- eventSender.mouseMoveTo(target.offsetLeft + 3, target.offsetTop + 3);
- }, function() {
- debug('Mouse down');
- eventSender.mouseDown();
- }, function() {
- debug('Mouse hold down, move');
- eventSender.mouseMoveTo(target.offsetLeft + 13, target.offsetTop + 3);
- }, function() {
- debug('Mouse up');
- eventSender.mouseUp();
- }
- ];
-
- var i = 0;
- function nextTest() {
- if (i < tests.length) {
- runTest(tests[i++], nextTest);
- } else {
- testsDone();
- }
- }
- nextTest();
-}
-
-</script>
-<script src=""
-</body>
-</html>
Deleted: trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt (142955 => 142956)
--- trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,16 +0,0 @@
-Test that there is no mousemove event fired when changing cursor.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-Bug 85343
-
-
-TEST CASE: Mouse idle, change cursor should not fire mousemove event
-Cursor Info: type=Pointer hotSpot=0,0
-Cursor Info: type=Help hotSpot=0,0
-
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove.html (142955 => 142956)
--- trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove.html 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove.html 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,55 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script src=""
-<style type="text/css">
-</style>
-</head>
-<body>
-<p id="description"></p>
-<p><a href="" 85343</a></p>
-<div id="test-container">
- <div id="target" style="cursor:default">Mouse idle, change cursor should not fire mousemove event</div>
-</div>
-<br/>
-<div id="console"></div>
-<script>
-description("Test that there is no mousemove event fired when changing cursor.");
-
-if (!window.eventSender) {
- testFailed('This test requires DumpRenderTree');
-}
-
-if (window.testRunner) {
- testRunner.dumpAsText();
- testRunner.waitUntilDone();
- window.jsTestIsAsync = true;
-}
-
-// Can't do anything useful here without eventSender
-if (window.eventSender) {
- var node = document.getElementById('target');
- debug('TEST CASE: ' + node.textContent);
- eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop + 3);
- debug('Cursor Info: ' + window.internals.getCurrentCursorInfo(document));
- node.addEventListener('mousemove', function() {
- testFailed('Mousemove event should not be fired when changing cursor');
- finishJSTest();
- });
- node.style.cursor = 'help';
- setTimeout(function() {
- debug('Cursor Info: ' + window.internals.getCurrentCursorInfo(document));
- debug('');
- }, 0);
-
- // Give some time for the change to resolve. Fake mousemove event that caused bug, is using a timer
- setTimeout(function() {
- document.getElementById('test-container').style.display = 'none';
- finishJSTest();
- }, 150);
-}
-
-</script>
-<script src=""
-</body>
-</html>
Modified: trunk/LayoutTests/platform/mac/TestExpectations (142955 => 142956)
--- trunk/LayoutTests/platform/mac/TestExpectations 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1416,7 +1416,4 @@
# Crashing after https://webkit.org/b/105667
webkit.org/b/109232 [ Debug ] inspector/debugger/debugger-reload-on-pause.html [ Crash ]
-# Mac fails cursor change test for unknown reasons
-webkit.org/b/103857 fast/events/mouse-cursor-change.html
-
webkit.org/b/109869 media/media-captions.html [ Crash ]
Modified: trunk/Source/WebCore/ChangeLog (142955 => 142956)
--- trunk/Source/WebCore/ChangeLog 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/Source/WebCore/ChangeLog 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1,3 +1,17 @@
+2013-02-14 Simon Fraser <simon.fra...@apple.com>
+
+ Reverting r142861. Hit testing inside of style recalc is fundamentally wrong
+
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::selectCursor):
+ (WebCore::EventHandler::handleMouseMoveEvent):
+ * page/EventHandler.h:
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::setStyle):
+ (WebCore::areNonIdenticalCursorListsEqual):
+ (WebCore::areCursorsEqual):
+ (WebCore::RenderObject::styleDidChange):
+
2013-02-14 Florin Malita <fmal...@chromium.org>
[SVG] Cached filter results are not invalidated on repaint rect change
@@ -1720,56 +1734,6 @@
* inspector/front-end/TimelinePanel.js:
* inspector/front-end/WebKit.qrc:
-2013-02-14 Aivo Paas <aivop...@gmail.com>
-
- Updating mouse cursor on style changes without emitting fake mousemove event
- https://bugs.webkit.org/show_bug.cgi?id=101857
-
- Reviewed by Allan Sandfeld Jensen.
-
- Mouse cursor changes in styles used to be reflected in UI through dispatching a fake
- mousemove event. The old approach has some flaws: it emits a mousemove event in
- _javascript_ when there is no mouse movement involved (bug 85343); the fake mousemove
- event is cancelled while there is a mouse button held down - cursor won't change
- until mouse is moved or the button released (bug 53341); it has extra overhead of
- using a timer which was introduced to make scrolling smoother.
-
- The new approach does not use the fake mousemove event. Instead, it uses only the logic
- needed for the actual cursor change to happen. This bypasses all the mousemove event related
- overhead. The remaining code is a stripped version of what was run through the mousemove
- event path. Everything that was not needed for changing a cursor is stripped off, everything
- that is needed, remains the same.
-
- The call to update cursor was moved up in the call tree from RenderObject::StyleDidChange
- to RenderObject::SetStyle right after the StyleDidChange call. This allows to any updates
- and style propagations in StyleDidChange to happen and makes sure that a cursor change is
- not missed. Previous place was at the end of RenderObject::StyleDidChange, where it could
- have been missed because of an early exit. For example, cursor change on mousedown/up on
- a text node missed the correct cursor in the first pass.
-
- Refactored EventHandler::selectCursor to not take a whole mouse event but instead work with
- HitTestResult so that EventHandler::updateCursor must not create a useless PlatformEvent.
-
- Fixes: https://bugs.webkit.org/show_bug.cgi?id=85343 (mousemove event on cursor change)
- https://bugs.webkit.org/show_bug.cgi?id=53341 (no cursor change when mouse button down)
-
- Tests: fast/events/mouse-cursor-change.html
- fast/events/mouse-cursor-no-mousemove.html
-
- * page/EventHandler.cpp:
- (WebCore::EventHandler::updateCursor): Newly added method for updating mouse cursor
- (WebCore):
- (WebCore::EventHandler::selectCursor):
- (WebCore::EventHandler::handleMouseMoveEvent):
- * page/EventHandler.h:
- (EventHandler):
- * rendering/RenderObject.cpp:
- (WebCore::areNonIdenticalCursorListsEqual):
- (WebCore):
- (WebCore::areCursorsEqual):
- (WebCore::RenderObject::setStyle):
- (WebCore::RenderObject::styleDidChange):
-
2013-02-13 Ilya Tikhonovsky <loi...@chromium.org>
Web Inspector: Native Memory Instrumentation: Report child nodes as direct members of a container node to make them look like a tree in the snapshot.
Modified: trunk/Source/WebCore/page/EventHandler.cpp (142955 => 142956)
--- trunk/Source/WebCore/page/EventHandler.cpp 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1237,41 +1237,8 @@
return ((isOverLink || isSubmitImage(node)) && (!editable || editableLinkEnabled));
}
-void EventHandler::updateCursor()
+OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& event, Scrollbar* scrollbar)
{
- if (m_mousePositionIsUnknown)
- return;
-
- Settings* settings = m_frame->settings();
- if (settings && !settings->deviceSupportsMouse())
- return;
-
- FrameView* view = m_frame->view();
- if (!view)
- return;
-
- if (!m_frame->page() || !m_frame->page()->isOnscreen() || !m_frame->page()->focusController()->isActive())
- return;
-
- bool shiftKey;
- bool ctrlKey;
- bool altKey;
- bool metaKey;
- PlatformKeyboardEvent::getCurrentModifierState(shiftKey, ctrlKey, altKey, metaKey);
-
- HitTestRequest request(HitTestRequest::ReadOnly);
- HitTestResult result(view->windowToContents(m_lastKnownMousePosition));
- m_frame->document()->renderView()->hitTest(request, result);
-
- OptionalCursor optionalCursor = selectCursor(result, shiftKey);
- if (optionalCursor.isCursorChange()) {
- m_currentMouseCursor = optionalCursor.cursor();
- view->setCursor(m_currentMouseCursor);
- }
-}
-
-OptionalCursor EventHandler::selectCursor(const HitTestResult& result, bool shiftKey)
-{
if (m_resizeLayer && m_resizeLayer->inResizeMode())
return NoCursorChange;
@@ -1283,16 +1250,8 @@
return NoCursorChange;
#endif
- Node* node = result.targetNode();
- if (!node)
- return NoCursorChange;
- bool originalIsText = node->isTextNode();
- if (node && originalIsText)
- node = node->parentNode();
- if (!node)
- return NoCursorChange;
-
- RenderObject* renderer = node->renderer();
+ Node* node = event.targetNode();
+ RenderObject* renderer = node ? node->renderer() : 0;
RenderStyle* style = renderer ? renderer->style() : 0;
bool horizontalText = !style || style->isHorizontalWritingMode();
const Cursor& iBeam = horizontalText ? iBeamCursor() : verticalTextCursor();
@@ -1308,7 +1267,7 @@
if (renderer) {
Cursor overrideCursor;
- switch (renderer->getCursor(roundedIntPoint(result.localPoint()), overrideCursor)) {
+ switch (renderer->getCursor(roundedIntPoint(event.localPoint()), overrideCursor)) {
case SetCursorBasedOnStyle:
break;
case SetCursor:
@@ -1355,19 +1314,19 @@
switch (style ? style->cursor() : CURSOR_AUTO) {
case CURSOR_AUTO: {
- bool editable = (node->rendererIsEditable());
+ bool editable = (node && node->rendererIsEditable());
- if (useHandCursor(node, result.URLElement() && result.URLElement()->isLink(), shiftKey))
+ if (useHandCursor(node, event.isOverLink(), event.event().shiftKey()))
return handCursor();
bool inResizer = false;
if (renderer) {
if (RenderLayer* layer = renderer->enclosingLayer()) {
if (FrameView* view = m_frame->view())
- inResizer = layer->isPointInResizeControl(view->windowToContents(roundedIntPoint(result.localPoint())));
+ inResizer = layer->isPointInResizeControl(view->windowToContents(event.event().position()));
}
}
- if ((editable || (originalIsText && node->canStartSelection())) && !inResizer && !result.scrollbar())
+ if ((editable || (renderer && renderer->isText() && node->canStartSelection())) && !inResizer && !scrollbar)
return iBeam;
return pointerCursor();
}
@@ -1778,7 +1737,7 @@
if (scrollbar && !m_mousePressed)
scrollbar->mouseMoved(mouseEvent); // Handle hover effects on platforms that support visual feedback on scrollbar hovering.
if (FrameView* view = m_frame->view()) {
- OptionalCursor optionalCursor = selectCursor(mev.hitTestResult(), mouseEvent.shiftKey());
+ OptionalCursor optionalCursor = selectCursor(mev, scrollbar);
if (optionalCursor.isCursorChange()) {
m_currentMouseCursor = optionalCursor.cursor();
view->setCursor(m_currentMouseCursor);
Modified: trunk/Source/WebCore/page/EventHandler.h (142955 => 142956)
--- trunk/Source/WebCore/page/EventHandler.h 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/Source/WebCore/page/EventHandler.h 2013-02-15 04:28:45 UTC (rev 142956)
@@ -251,7 +251,6 @@
#endif
bool useHandCursor(Node*, bool isOverLink, bool shiftKey);
- void updateCursor();
private:
#if ENABLE(DRAG_SUPPORT)
@@ -278,8 +277,7 @@
#endif
bool handleMouseReleaseEvent(const MouseEventWithHitTestResults&);
- OptionalCursor selectCursor(const HitTestResult&, bool shiftKey);
-
+ OptionalCursor selectCursor(const MouseEventWithHitTestResults&, Scrollbar*);
void hoverTimerFired(Timer<EventHandler>*);
bool logicalScrollOverflow(ScrollLogicalDirection, ScrollGranularity, Node* startingNode = 0);
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (142955 => 142956)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2013-02-15 03:58:18 UTC (rev 142955)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2013-02-15 04:28:45 UTC (rev 142956)
@@ -1758,17 +1758,6 @@
setStyle(pseudoStyle);
}
-static bool areNonIdenticalCursorListsEqual(const RenderStyle* a, const RenderStyle* b)
-{
- ASSERT(a->cursors() != b->cursors());
- return a->cursors() && b->cursors() && *a->cursors() == *b->cursors();
-}
-
-static inline bool areCursorsEqual(const RenderStyle* a, const RenderStyle* b)
-{
- return a->cursor() == b->cursor() && (a->cursors() == b->cursors() || areNonIdenticalCursorListsEqual(a, b));
-}
-
void RenderObject::setStyle(PassRefPtr<RenderStyle> style)
{
if (m_style == style) {
@@ -1807,11 +1796,6 @@
styleDidChange(diff, oldStyle.get());
- if (oldStyle.get() && !areCursorsEqual(oldStyle.get(), this->style())) {
- if (Frame* frame = this->frame())
- frame->eventHandler()->updateCursor();
- }
-
// FIXME: |this| might be destroyed here. This can currently happen for a RenderTextFragment when
// its first-letter block gets an update in RenderTextFragment::styleDidChange. For RenderTextFragment(s),
// we will safely bail out with the doesNotNeedLayout flag. We might want to broaden this condition
@@ -1937,6 +1921,17 @@
}
}
+static bool areNonIdenticalCursorListsEqual(const RenderStyle* a, const RenderStyle* b)
+{
+ ASSERT(a->cursors() != b->cursors());
+ return a->cursors() && b->cursors() && *a->cursors() == *b->cursors();
+}
+
+static inline bool areCursorsEqual(const RenderStyle* a, const RenderStyle* b)
+{
+ return a->cursor() == b->cursor() && (a->cursors() == b->cursors() || areNonIdenticalCursorListsEqual(a, b));
+}
+
void RenderObject::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
{
@@ -1970,6 +1965,11 @@
// Don't check for repaint here; we need to wait until the layer has been
// updated by subclasses before we know if we have to repaint (in setStyle()).
+
+ if (oldStyle && !areCursorsEqual(oldStyle, style())) {
+ if (Frame* frame = this->frame())
+ frame->eventHandler()->dispatchFakeMouseMoveEventSoon();
+ }
}
void RenderObject::propagateStyleToAnonymousChildren(bool blockChildrenOnly)
