Bug 17980: Regression: Inspector highlighting of webpage not cleared when going to...
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Mar 2008 16:00:41 +0000 (16:00 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Mar 2008 16:00:41 +0000 (16:00 +0000)
http://bugs.webkit.org/show_bug.cgi?id=17980

Reviewed by Adam.

The new highlight drawing was not honoring the fade value, so it was
always drawing at full opacity. The animation code didn't match Windows
and the new highlight anyway, so it has been removed. The highlight
how just detaches when it is hidden.

* WebCoreSupport/WebInspectorClient.mm:
(-[WebInspectorWindowController windowShouldClose:]): Call hideHighlight.
(-[WebInspectorWindowController close]): Ditto.
(-[WebInspectorWindowController highlightNode:]): Call attach.
(-[WebInspectorWindowController hideHighlight]): Call detach and release _currentHighlight.
* WebInspector/WebNodeHighlight.h:
* WebInspector/WebNodeHighlight.m:
(-[WebNodeHighlight initWithTargetView:inspectorController:]):
(-[WebNodeHighlight dealloc]): Assert we have no _highlightView.
(-[WebNodeHighlight attach]): Renamed from attachHighlight.
(-[WebNodeHighlight detach]): Renamed from detachHighlight.
(-[WebNodeHighlight setNeedsUpdateInTargetViewRect:]): Renamed from setHolesNeedUpdateInTargetViewRect:.
* WebInspector/WebNodeHighlightView.h:
* WebInspector/WebNodeHighlightView.m:
(-[WebNodeHighlightView setNeedsDisplayInRect:]): Renamed from setHolesNeedUpdateInRect:.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31209 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKit/mac/ChangeLog
WebKit/mac/WebCoreSupport/WebInspectorClient.mm
WebKit/mac/WebInspector/WebNodeHighlight.h
WebKit/mac/WebInspector/WebNodeHighlight.m
WebKit/mac/WebInspector/WebNodeHighlightView.h
WebKit/mac/WebInspector/WebNodeHighlightView.m

index 190f446..4560770 100644 (file)
@@ -1,3 +1,31 @@
+2008-03-21  Timothy Hatcher  <timothy@apple.com>
+
+        Bug 17980: Regression: Inspector highlighting of webpage not cleared when going to new URL
+        http://bugs.webkit.org/show_bug.cgi?id=17980
+
+        Reviewed by Adam.
+
+        The new highlight drawing was not honoring the fade value, so it was
+        always drawing at full opacity. The animation code didn't match Windows
+        and the new highlight anyway, so it has been removed. The highlight
+        how just detaches when it is hidden.
+
+        * WebCoreSupport/WebInspectorClient.mm:
+        (-[WebInspectorWindowController windowShouldClose:]): Call hideHighlight.
+        (-[WebInspectorWindowController close]): Ditto.
+        (-[WebInspectorWindowController highlightNode:]): Call attach.
+        (-[WebInspectorWindowController hideHighlight]): Call detach and release _currentHighlight.
+        * WebInspector/WebNodeHighlight.h:
+        * WebInspector/WebNodeHighlight.m:
+        (-[WebNodeHighlight initWithTargetView:inspectorController:]):
+        (-[WebNodeHighlight dealloc]): Assert we have no _highlightView.
+        (-[WebNodeHighlight attach]): Renamed from attachHighlight.
+        (-[WebNodeHighlight detach]): Renamed from detachHighlight.
+        (-[WebNodeHighlight setNeedsUpdateInTargetViewRect:]): Renamed from setHolesNeedUpdateInTargetViewRect:.
+        * WebInspector/WebNodeHighlightView.h:
+        * WebInspector/WebNodeHighlightView.m:
+        (-[WebNodeHighlightView setNeedsDisplayInRect:]): Renamed from setHolesNeedUpdateInRect:.
+
 2008-03-20  Mark Rowe  <mrowe@apple.com>
 
         Reviewed by Sam Weinig.
index 6fc3c76..1c6584b 100644 (file)
@@ -200,6 +200,7 @@ void WebInspectorClient::updateWindowTitle() const
 
 - (void)dealloc
 {
+    ASSERT(!_currentHighlight);
     [_shadowView release];
     [_webView release];
     [super dealloc];
@@ -254,10 +255,7 @@ void WebInspectorClient::updateWindowTitle() const
 
     [_inspectedWebView page]->inspectorController()->setWindowVisible(false);
 
-    [_currentHighlight detachHighlight];
-    [_currentHighlight setDelegate:nil];
-    [_currentHighlight release];
-    _currentHighlight = nil;
+    [self hideHighlight];
 
     return YES;
 }
@@ -271,12 +269,8 @@ void WebInspectorClient::updateWindowTitle() const
 
     [_inspectedWebView page]->inspectorController()->setWindowVisible(false);
 
-    if (!_movingWindows) {
-        [_currentHighlight detachHighlight];
-        [_currentHighlight setDelegate:nil];
-        [_currentHighlight release];
-        _currentHighlight = nil;
-    }
+    if (!_movingWindows)
+        [self hideHighlight];
 
     if (_attachedToInspectedWebView) {
         if ([_inspectedWebView _isClosed])
@@ -470,20 +464,19 @@ void WebInspectorClient::updateWindowTitle() const
     if (!_currentHighlight) {
         _currentHighlight = [[WebNodeHighlight alloc] initWithTargetView:view inspectorController:[_inspectedWebView page]->inspectorController()];
         [_currentHighlight setDelegate:self];
-        [_currentHighlight attachHighlight];
+        [_currentHighlight attach];
     }
 
-    [_currentHighlight show];
-
     // FIXME: this is a hack until we hook up a didDraw and didScroll call in WebHTMLView
     [[_currentHighlight highlightView] setNeedsDisplay:YES];
 }
 
 - (void)hideHighlight
 {
-    if (!_currentHighlight)
-        return;
-    [_currentHighlight hide];
+    [_currentHighlight detach];
+    [_currentHighlight setDelegate:nil];
+    [_currentHighlight release];
+    _currentHighlight = nil;
 }
 
 #pragma mark -
index a8e8748..1eb277a 100644 (file)
@@ -36,7 +36,6 @@ namespace WebCore {
     NSView *_targetView;
     NSWindow *_highlightWindow;
     WebNodeHighlightView *_highlightView;
-    NSAnimation *_fadeInAnimation;
     WebCore::InspectorController* _inspectorController;
     id _delegate;
 }
@@ -45,22 +44,15 @@ namespace WebCore {
 - (void)setDelegate:(id)delegate;
 - (id)delegate;
 
-- (void)attachHighlight;
-- (void)detachHighlight;
-
-- (void)show;
-- (void)hide;
+- (void)attach;
+- (void)detach;
 
 - (NSView *)targetView;
 - (WebNodeHighlightView *)highlightView;
 
 - (WebCore::InspectorController*)inspectorController;
 
-// Controls whether mouse events are ignored (passed to underlying view). By default mouse events are ignored.
-- (BOOL)ignoresMouseEvents;
-- (void)setIgnoresMouseEvents:(BOOL)newValue;
-
-- (void)setHolesNeedUpdateInTargetViewRect:(NSRect)rect;
+- (void)setNeedsUpdateInTargetViewRect:(NSRect)rect;
 @end
 
 @interface NSObject (WebNodeHighlightDelegate)
index b4d088a..82e09f1 100644 (file)
 
 using namespace WebCore;
 
-#define FADE_ANIMATION_DURATION 0.2
-
-@interface WebNodeHighlightFadeInAnimation : NSAnimation
-@end
-
 @interface WebNodeHighlight (FileInternal)
 - (NSRect)_computeHighlightWindowFrame;
 - (void)_repositionHighlightWindow;
-- (void)_animateFadeIn:(WebNodeHighlightFadeInAnimation *)animation;
-@end
-
-@implementation WebNodeHighlightFadeInAnimation
-
-- (void)setCurrentProgress:(NSAnimationProgress)progress
-{
-    [super setCurrentProgress:progress];
-    [(WebNodeHighlight *)[self delegate] _animateFadeIn:self];
-}
-
 @end
 
 @implementation WebNodeHighlight
@@ -76,7 +60,6 @@ using namespace WebCore;
     [_highlightWindow setReleasedWhenClosed:NO];
 
     _highlightView = [[WebNodeHighlightView alloc] initWithWebNodeHighlight:self];
-    [_highlightView setFractionFadedIn:0.0];
     [_highlightWindow setContentView:_highlightView];
     [_highlightView release];
 
@@ -85,26 +68,22 @@ using namespace WebCore;
 
 - (void)dealloc
 {
-    // FIXME: Bad to do all this work in dealloc. What about under GC?
-
-    [self detachHighlight];
-
     ASSERT(!_highlightWindow);
     ASSERT(!_targetView);
-
-    [_fadeInAnimation setDelegate:nil];
-    [_fadeInAnimation stopAnimation];
-    [_fadeInAnimation release];
+    ASSERT(!_highlightView);
 
     [super dealloc];
 }
 
-- (void)attachHighlight
+- (void)attach
 {
     ASSERT(_targetView);
     ASSERT([_targetView window]);
     ASSERT(_highlightWindow);
 
+    if (!_highlightWindow || !_targetView || ![_targetView window])
+        return;
+
     // Disable screen updates so the highlight moves in sync with the view.
     [[_targetView window] disableScreenUpdatesUntilFlush];
     [[_targetView window] addChildWindow:_highlightWindow ordered:NSWindowAbove];
@@ -128,7 +107,7 @@ using namespace WebCore;
     return _delegate;
 }
 
-- (void)detachHighlight
+- (void)detach
 {
     if (!_highlightWindow) {
         ASSERT(!_targetView);
@@ -159,36 +138,6 @@ using namespace WebCore;
     _highlightView = nil;
 }
 
-- (void)show
-{
-    ASSERT(!_fadeInAnimation);
-    if (_fadeInAnimation || [_highlightView fractionFadedIn] == 1.0)
-        return;
-
-    _fadeInAnimation = [[WebNodeHighlightFadeInAnimation alloc] initWithDuration:FADE_ANIMATION_DURATION animationCurve:NSAnimationEaseInOut];
-    [_fadeInAnimation setAnimationBlockingMode:NSAnimationNonblocking];
-    [_fadeInAnimation setDelegate:self];
-    [_fadeInAnimation startAnimation];
-}
-
-- (void)hide
-{
-    [_highlightView setFractionFadedIn:0.0];
-}
-
-- (void)animationDidEnd:(NSAnimation *)animation
-{
-    ASSERT(animation == _fadeInAnimation);
-    [_fadeInAnimation release];
-    _fadeInAnimation = nil;
-}
-
-- (BOOL)ignoresMouseEvents
-{
-    ASSERT(_highlightWindow);
-    return [_highlightWindow ignoresMouseEvents];
-}
-
 - (WebNodeHighlightView *)highlightView
 {
     return _highlightView;
@@ -200,11 +149,11 @@ using namespace WebCore;
     _delegate = delegate;
 }
 
-- (void)setHolesNeedUpdateInTargetViewRect:(NSRect)rect
+- (void)setNeedsUpdateInTargetViewRect:(NSRect)rect
 {
     ASSERT(_targetView);
 
-    [_highlightView setHolesNeedUpdateInRect:[_targetView _web_convertRect:rect toView:_highlightView]];
+    [_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
@@ -212,12 +161,6 @@ using namespace WebCore;
     [_highlightView displayIfNeeded];
 }
 
-- (void)setIgnoresMouseEvents:(BOOL)newValue
-{
-    ASSERT(_highlightWindow);
-    [_highlightWindow setIgnoresMouseEvents:newValue];
-}
-
 - (NSView *)targetView
 {
     return _targetView;
@@ -253,9 +196,4 @@ using namespace WebCore;
     [_highlightWindow setFrame:[self _computeHighlightWindowFrame] display:YES];
 }
 
-- (void)_animateFadeIn:(WebNodeHighlightFadeInAnimation *)animation
-{
-    [_highlightView setFractionFadedIn:[animation currentValue]];
-}
-
 @end
index 70d1a5a..cb317b8 100644 (file)
 
 @interface WebNodeHighlightView : NSView {
     WebNodeHighlight *_webNodeHighlight;
-    float _fractionFadedIn;
 }
 - (id)initWithWebNodeHighlight:(WebNodeHighlight *)webNodeHighlight;
 
 - (WebNodeHighlight *)webNodeHighlight;
 - (void)detachFromWebNodeHighlight;
-
-// Value between 0.0 (completely faded out of view) and 1.0 (completely faded into view) that represents
-// the progress of the fading animation.
-- (float)fractionFadedIn;
-- (void)setFractionFadedIn:(float)alpha;
-
-- (void)setHolesNeedUpdateInRect:(NSRect)rect;
 @end
index 9f40009..fb8cc77 100644 (file)
@@ -42,7 +42,7 @@ using namespace WebCore;
 #define OVERLAY_MAX_ALPHA 0.7
 #define OVERLAY_WHITE_VALUE 0.1
 
-#define WHITE_FRAME_THICKNESS 1.0
+#define BORDER_THICKNESS 1.0
 
 @implementation WebNodeHighlightView
 
@@ -91,29 +91,11 @@ using namespace WebCore;
     return _webNodeHighlight;
 }
 
-- (float)fractionFadedIn
+- (void)setNeedsDisplayInRect:(NSRect)rect
 {
-    return _fractionFadedIn;
-}
-
-- (void)setFractionFadedIn:(float)fraction
-{
-    ASSERT_ARG(fraction, fraction >= 0.0 && fraction <= 1.0);
-
-    if (_fractionFadedIn == fraction)
-        return;
-    
-    _fractionFadedIn = fraction;
-    [self setNeedsDisplay:YES];
-}
-
-- (void)setHolesNeedUpdateInRect:(NSRect)rect
-{
-    // Redisplay a slightly larger rect to account for white border around holes
-    rect = NSInsetRect(rect, -1 * WHITE_FRAME_THICKNESS, 
-                             -1 * WHITE_FRAME_THICKNESS);
-
-    [self setNeedsDisplayInRect:rect];
+    // Redisplay a slightly larger rect to account for the border
+    rect = NSInsetRect(rect, -1 * BORDER_THICKNESS, -1 * BORDER_THICKNESS);
+    [super setNeedsDisplayInRect:rect];
 }
 
 @end