Changeset 37046 in webkit

Timestamp:

Sep 28, 2008 4:31:25 PM (16 years ago)

Author:

timothy@apple.com

Message:

Improves the Web Inspector node highlight so it does not scroll to reveal the node in the page.

WebCore:

2008-09-28 Timothy Hatcher <​timothy@apple.com>

Remove the scrollIntoViewIfNeeded() call when drawing the
Inspector node highlight.

<rdar://problem/6115804> Don't scroll when highlighting (21000)
​https://bugs.webkit.org/show_bug.cgi?id=21000

Reviewed by Dan Bernstein.

• page/InspectorController.cpp: (WebCore::InspectorController::drawNodeHighlight):

WebKit/mac:

2008-09-28 Timothy Hatcher <​timothy@apple.com>

Improves the Web Inspector node highlight so it does not scroll
to reveal the node in the page. This makes the highlight less
invasive and causes less things to change on screen. Also makes
the highlight redraw when the WebView draws, so it stays current
if the node changes on the page for any reason.

<rdar://problem/6115804> Don't scroll when highlighting (21000)
​https://bugs.webkit.org/show_bug.cgi?id=21000

Reviewed by Dan Bernstein.

• WebCoreSupport/WebInspectorClient.mm: (-[WebInspectorWindowController highlightNode:]): Call setNeedsDisplay:YES if there is an existing highlight. (-[WebInspectorWindowController didAttachWebNodeHighlight:]): Set the current highlight node on the inspected WebView. (-[WebInspectorWindowController willDetachWebNodeHighlight:]): Set the current highlight node on the inspected WebView to nil.
• WebInspector/WebNodeHighlight.m: (-[WebNodeHighlight setNeedsUpdateInTargetViewRect:]): Disable screen updates until flush for the inspected window. Invalidate the whole highlight view since we don't know the rect that needs updated since the highlight can be larger than the highlighted element due to the margins and other factors.
• WebInspector/WebNodeHighlightView.m:
• WebView/WebHTMLView.mm: (-[WebHTMLView drawSingleRect:]): Call setNeedsUpdateInTargetViewRect: on the current highlight node.
• WebView/WebView.mm: (-[WebViewPrivate dealloc]): Release the current highlight node. (-[WebView setCurrentNodeHighlight:]): Set the current highlight node. (-[WebView currentNodeHighlight]): Return the current highlight node.
• WebView/WebViewInternal.h:

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/InspectorController.cpp (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebCoreSupport/WebInspectorClient.mm (modified) (6 diffs)
• WebKit/mac/WebInspector/WebNodeHighlight.m (modified) (2 diffs)
• WebKit/mac/WebInspector/WebNodeHighlightView.m (modified) (3 diffs)
• WebKit/mac/WebView/WebHTMLView.mm (modified) (2 diffs)
• WebKit/mac/WebView/WebView.mm (modified) (4 diffs)
• WebKit/mac/WebView/WebViewInternal.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-09-28  Timothy Hatcher  <timothy@apple.com>
+
+        Remove the scrollIntoViewIfNeeded() call when drawing the
+        Inspector node highlight.
+
+        <rdar://problem/6115804> Don't scroll when highlighting (21000)
+        https://bugs.webkit.org/show_bug.cgi?id=21000
+
+        Reviewed by Dan Bernstein.
+
+        * page/InspectorController.cpp:
+        (WebCore::InspectorController::drawNodeHighlight):
+
 2008-09-28  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/page/InspectorController.cpp

@@ -2500 +2500 @@
         else
             element = static_cast<Element*>(m_highlightedNode->parent());
-        element->scrollIntoViewIfNeeded();
         overlayRect = view->visibleContentRect();
     }

trunk/WebKit/mac/ChangeLog

@@ +1 @@
+2008-09-28  Timothy Hatcher  <timothy@apple.com>
+
+        Improves the Web Inspector node highlight so it does not scroll
+        to reveal the node in the page. This makes the highlight less
+        invasive and causes less things to change on screen. Also makes
+        the highlight redraw when the WebView draws, so it stays current
+        if the node changes on the page for any reason.
+
+        <rdar://problem/6115804> Don't scroll when highlighting (21000)
+        https://bugs.webkit.org/show_bug.cgi?id=21000
+
+        Reviewed by Dan Bernstein.
+
+        * WebCoreSupport/WebInspectorClient.mm:
+        (-[WebInspectorWindowController highlightNode:]): Call setNeedsDisplay:YES
+        if there is an existing highlight.
+        (-[WebInspectorWindowController didAttachWebNodeHighlight:]): Set the current
+        highlight node on the inspected WebView.
+        (-[WebInspectorWindowController willDetachWebNodeHighlight:]): Set the current
+        highlight node on the inspected WebView to nil.
+        * WebInspector/WebNodeHighlight.m:
+        (-[WebNodeHighlight setNeedsUpdateInTargetViewRect:]): Disable screen updates
+        until flush for the inspected window. Invalidate the whole highlight view since
+        we don't know the rect that needs updated since the highlight can be larger than
+        the highlighted element due to the margins and other factors.
+        * WebInspector/WebNodeHighlightView.m:
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView drawSingleRect:]): Call setNeedsUpdateInTargetViewRect: on the
+        current highlight node.
+        * WebView/WebView.mm:
+        (-[WebViewPrivate dealloc]): Release the current highlight node.
+        (-[WebView setCurrentNodeHighlight:]): Set the current highlight node.
+        (-[WebView currentNodeHighlight]): Return the current highlight node.
+        * WebView/WebViewInternal.h:
+
 2008-09-28  David Kilzer  <ddkilzer@apple.com>
 

trunk/WebKit/mac/WebCoreSupport/WebInspectorClient.mm

@@ -1 +1 @@
 /*
- * Copyright (C) 2006, 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006, 2007, 2008 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -62 +62 @@
 - (void)detach;
 - (void)setAttachedWindowHeight:(unsigned)height;
-- (void)highlightAndScrollToNode:(DOMNode *)node;
 - (void)highlightNode:(DOMNode *)node;
 - (void)hideHighlight;
@@ -124 +123 @@
 void WebInspectorClient::highlight(Node* node)
 {
-    [m_windowController.get() highlightAndScrollToNode:kit(node)];
+    [m_windowController.get() highlightNode:kit(node)];
 }
 
@@ -393 +392 @@
 #pragma mark -
 
-- (void)highlightAndScrollToNode:(DOMNode *)node
-{
-    NSRect bounds = [node boundingBox];
-    if (!NSIsEmptyRect(bounds)) {
-        // FIXME: this needs to use the frame the node coordinates are in
-        NSRect visible = [[[[_inspectedWebView mainFrame] frameView] documentView] visibleRect];
-        BOOL needsScroll = !NSContainsRect(visible, bounds) && !NSContainsRect(bounds, visible);
-
-        // only scroll if the bounds isn't in the visible rect and dosen't contain the visible rect
-        if (needsScroll) {
-            // scroll to the parent element if we aren't focused on an element
-            DOMElement *element;
-            if ([node isKindOfClass:[DOMElement class]])
-                element = (DOMElement *)node;
-            else
-                element = (DOMElement *)[node parentNode];
-            [element scrollIntoViewIfNeeded:YES];
-
-            // give time for the scroll to happen
-            [self performSelector:@selector(highlightNode:) withObject:node afterDelay:0.25];
-        } else
-            [self highlightNode:node];
-    }
-}
-
 - (void)highlightNode:(DOMNode *)node
 {
@@ -429 +403 @@
         [_currentHighlight setDelegate:self];
         [_currentHighlight attach];
-    }
-
-    // FIXME: this is a hack until we hook up a didDraw and didScroll call in WebHTMLView
-    [[_currentHighlight highlightView] setNeedsDisplay:YES];
+    } else
+        [[_currentHighlight highlightView] setNeedsDisplay:YES];
 }
 
@@ -441 +413 @@
     [_currentHighlight release];
     _currentHighlight = nil;
+}
+
+#pragma mark -
+#pragma mark WebNodeHighlight delegate
+
+- (void)didAttachWebNodeHighlight:(WebNodeHighlight *)highlight
+{
+    [_inspectedWebView setCurrentNodeHighlight:highlight];
+}
+
+- (void)willDetachWebNodeHighlight:(WebNodeHighlight *)highlight
+{
+    [_inspectedWebView setCurrentNodeHighlight:nil];
 }
 

trunk/WebKit/mac/WebInspector/WebNodeHighlight.m

@@ -1 +1 @@
 /*
- * Copyright (C) 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2007, 2008 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -149 +149 @@
     ASSERT(_targetView);
 
-    [_highlightView setNeedsDisplayInRect:[_targetView _web_convertRect:rect toView:_highlightView]];
-
-    // Redraw highlight view immediately so it updates in sync with the target view
-    // if we called disableScreenUpdatesUntilFlush on the target view earlier. This
-    // is especially visible when resizing the window.
+    [[_targetView window] disableScreenUpdatesUntilFlush];
+
+    // Mark the whole highlight view as needing display since we don't know what areas
+    // need updated, since the highlight can be larger than the element to show margins.
+    [_highlightView setNeedsDisplay:YES];
+
+    // Redraw highlight view immediately so it updates in sync with the target view.
+    // This is especially visible when resizing the window, scrolling or with DHTML.
     [_highlightView displayIfNeeded];
 }

trunk/WebKit/mac/WebInspector/WebNodeHighlightView.m

@@ -1 +1 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2006, 2008 Apple Computer, Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -35 +35 @@
 
 using namespace WebCore;
-
-#define OVERLAY_MAX_ALPHA 0.7
-#define OVERLAY_WHITE_VALUE 0.1
-
-#define BORDER_THICKNESS 1.0
 
 @implementation WebNodeHighlightView
@@ -88 +83 @@
 }
 
-- (void)setNeedsDisplayInRect:(NSRect)rect
-{
-    // Redisplay a slightly larger rect to account for the border
-    rect = NSInsetRect(rect, -1 * BORDER_THICKNESS, -1 * BORDER_THICKNESS);
-    [super setNeedsDisplayInRect:rect];
-}
-
 @end

trunk/WebKit/mac/WebView/WebHTMLView.mm

@@ -52 +52 @@
 #import "WebKitVersionChecks.h"
 #import "WebLocalizableStrings.h"
+#import "WebNodeHighlight.h"
 #import "WebNSAttributedStringExtras.h"
 #import "WebNSEventExtras.h"
@@ -2938 +2939 @@
         [[self _frame] _drawRect:rect];
 
+        WebView *webView = [self _webView];
+
         // This hack is needed for <rdar://problem/5023545>. We can hit a race condition where drawRect will be
         // called after the WebView has closed. If the client did not properly close the WebView and set the 
         // UIDelegate to nil, then the UIDelegate will be stale and this code will crash. 
         static BOOL version3OrLaterClient = WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITHOUT_QUICKBOOKS_QUIRK);
-        if (version3OrLaterClient) {
-            WebView *webView = [self _webView];
+        if (version3OrLaterClient)
             [[webView _UIDelegateForwarder] webView:webView didDrawRect:[webView convertRect:rect fromView:self]];
-        }
+
+        if (WebNodeHighlight *currentHighlight = [webView currentNodeHighlight])
+            [currentHighlight setNeedsUpdateInTargetViewRect:[self convertRect:rect toView:[currentHighlight targetView]]];
 
         [(WebClipView *)[self superview] resetAdditionalClip];

trunk/WebKit/mac/WebView/WebView.mm

@@ -66 +66 @@
 #import "WebKitVersionChecks.h"
 #import "WebLocalizableStrings.h"
+#import "WebNodeHighlight.h"
 #import "WebNSDataExtras.h"
 #import "WebNSDataExtrasPrivate.h"
@@ -329 +330 @@
 
     WebInspector *inspector;
+    WebNodeHighlight *currentNodeHighlight;
 
     BOOL allowsUndo;
@@ -512 +514 @@
     
     [inspector release];
+    [currentNodeHighlight release];
+
     [hostWindow release];
 
@@ -2942 +2946 @@
 }
 
+- (void)setCurrentNodeHighlight:(WebNodeHighlight *)nodeHighlight
+{
+    id old = _private->currentNodeHighlight;
+    _private->currentNodeHighlight = [nodeHighlight retain];
+    [old release];
+}
+
+- (WebNodeHighlight *)currentNodeHighlight
+{
+    return _private->currentNodeHighlight;
+}
+
 @end
 

trunk/WebKit/mac/WebView/WebViewInternal.h

@@ -53 +53 @@
 @class WebBasePluginPackage;
 @class WebDownload;
+@class WebNodeHighlight;
 
 @interface WebView (WebViewEditingExtras)
@@ -114 +115 @@
 - (BOOL)_isMIMETypeRegisteredAsPlugin:(NSString *)MIMEType;
 
+- (void)setCurrentNodeHighlight:(WebNodeHighlight *)nodeHighlight;
+- (WebNodeHighlight *)currentNodeHighlight;
+
 - (void)addPluginInstanceView:(NSView *)view;
 - (void)removePluginInstanceView:(NSView *)view;