[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

darin at apple.com darin at apple.com
Mon Feb 21 00:34:31 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 88b13e65d7539cb75281e7f310e465658fbf1a3d
Author: darin at apple.com <darin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Feb 1 22:49:01 2011 +0000

    2011-02-01  Darin Adler  <darin at apple.com>
    
            Reviewed by Chris Fleizach.
    
            REGRESSION: Removing focus from area element causes unwanted scrolling
            https://bugs.webkit.org/show_bug.cgi?id=50169
    
            Test: fast/images/imagemap-scroll.html
    
            * html/HTMLAreaElement.cpp:
            (WebCore::HTMLAreaElement::setFocus): Added override. Calls the new
            RenderImage::areaElementFocusChanged function.
            (WebCore::HTMLAreaElement::updateFocusAppearance): Removed the code
            here that calls setNeedsLayout on the image's renderer. This was an
            attempt to cause repaint of the renderer, but this function does not
            need to do that. Also changed this to use the imageElement function
            to avoid repeating code.
    
            * html/HTMLAreaElement.h: Updated for above changes.
    
            * rendering/RenderImage.cpp:
            (WebCore::RenderImage::paint): Updated for name change.
            (WebCore::RenderImage::paintAreaElementFocusRing): Renamed this from
            paintFocusRing, because it only paints area focus rings, and should
            not be confused with paintFocusRing functions in other classes. Also
            removed the unused style argument. Removed the code that used an
            HTMLCollection to see if the focused area element is for this image
            and instead just call imageElement on the area element.
            (WebCore::RenderImage::areaElementFocusChanged): Added. Calls repaint.
    
            * rendering/RenderImage.h: Added a public areaElementFocusChanged
            function for HTMLAreaElement to call. Made the paintFocusRing function
            private, renamed it to paintAreaElementFocusRing, and removed its
            unused style argument.
    2011-02-01  Darin Adler  <darin at apple.com>
    
            Reviewed by Chris Fleizach.
    
            REGRESSION: Removing focus from area element causes unwanted scrolling
            https://bugs.webkit.org/show_bug.cgi?id=50169
    
            * fast/images/imagemap-scroll-expected.txt: Added.
            * fast/images/imagemap-scroll.html: Added.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77313 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5349312..f18a5fd 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2011-02-01  Darin Adler  <darin at apple.com>
+
+        Reviewed by Chris Fleizach.
+
+        REGRESSION: Removing focus from area element causes unwanted scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=50169
+
+        * fast/images/imagemap-scroll-expected.txt: Added.
+        * fast/images/imagemap-scroll.html: Added.
+
 2011-02-01  Zhenyao Mo  <zmo at google.com>
 
         Reviewed by Kenneth Russell.
diff --git a/LayoutTests/fast/images/imagemap-scroll-expected.txt b/LayoutTests/fast/images/imagemap-scroll-expected.txt
new file mode 100644
index 0000000..ced569a
--- /dev/null
+++ b/LayoutTests/fast/images/imagemap-scroll-expected.txt
@@ -0,0 +1,11 @@
+This tests to be sure that focusing an area element triggers scrolling and removing focus from it does not.
+
+PASS: Document is starting scrolled to top.
+
+PASS: Focusing area element caused the image to scroll into view.
+
+PASS: Document is scrolled to top once again.
+
+PASS: Document is still scrolled to top after removing focus from area element.
+
+TEST COMPLETE
diff --git a/LayoutTests/fast/images/imagemap-scroll.html b/LayoutTests/fast/images/imagemap-scroll.html
new file mode 100644
index 0000000..833f6d1
--- /dev/null
+++ b/LayoutTests/fast/images/imagemap-scroll.html
@@ -0,0 +1,53 @@
+<html>
+<head>
+<script>
+function log(message)
+{
+    var element = document.createElement("p");
+    element.appendChild(document.createTextNode(message));
+    document.getElementById("results").appendChild(element);
+}
+function runTest()
+{
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    if (document.body.scrollTop == 0)
+        log("PASS: Document is starting scrolled to top.");
+    else
+        log("FAIL: Document is starting scrolled to " + document.body.scrollTop + ".");
+    document.getElementById("area").focus();
+    if (document.body.scrollTop > 4000 && document.body.scrollTop < 6000)
+        log("PASS: Focusing area element caused the image to scroll into view.");
+    else
+        log("FAIL: Document is scrolled to " + document.body.scrollTop + " after focusing area element.");
+    document.body.scrollTop = 0;
+    if (document.body.scrollTop == 0)
+        log("PASS: Document is scrolled to top once again.");
+    else
+        log("FAIL: Document should be scrolled to top but is scrolled to " + document.body.scrollTop + ".");
+    document.getElementById("area").blur();
+    if (document.body.scrollTop == 0)
+        log("PASS: Document is still scrolled to top after removing focus from area element.");
+    else
+        log("FAIL: Document is scrolled to " + document.body.scrollTop + " after removing focus from area element.");
+    document.body.scrollTop = 0;
+    document.body.removeChild(document.getElementById("test"));
+    log("TEST COMPLETE");
+}
+</script>
+</head>
+<body onload="runTest()">
+<div id="results">
+<p>This tests to be sure that focusing an area element triggers scrolling and removing focus from it does not.</p>
+</div>
+<div id="test">
+<div style="height:5000px"></div>
+<map name="imagemap">
+    <area id="area" shape="rect" coords="0,0,128,128" href="#dummy">
+</map>
+<img src="resources/mu.png" width="128" height="128" usemap="#imagemap" ismap>
+<div style="height:5000px"></div>
+<div>
+</body>
+</head>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ec7da28..5052312 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2011-02-01  Darin Adler  <darin at apple.com>
+
+        Reviewed by Chris Fleizach.
+
+        REGRESSION: Removing focus from area element causes unwanted scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=50169
+
+        Test: fast/images/imagemap-scroll.html
+
+        * html/HTMLAreaElement.cpp:
+        (WebCore::HTMLAreaElement::setFocus): Added override. Calls the new
+        RenderImage::areaElementFocusChanged function.
+        (WebCore::HTMLAreaElement::updateFocusAppearance): Removed the code
+        here that calls setNeedsLayout on the image's renderer. This was an
+        attempt to cause repaint of the renderer, but this function does not
+        need to do that. Also changed this to use the imageElement function
+        to avoid repeating code.
+
+        * html/HTMLAreaElement.h: Updated for above changes.
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paint): Updated for name change.
+        (WebCore::RenderImage::paintAreaElementFocusRing): Renamed this from
+        paintFocusRing, because it only paints area focus rings, and should
+        not be confused with paintFocusRing functions in other classes. Also
+        removed the unused style argument. Removed the code that used an
+        HTMLCollection to see if the focused area element is for this image
+        and instead just call imageElement on the area element.
+        (WebCore::RenderImage::areaElementFocusChanged): Added. Calls repaint.
+
+        * rendering/RenderImage.h: Added a public areaElementFocusChanged
+        function for HTMLAreaElement to call. Made the paintFocusRing function
+        private, renamed it to paintAreaElementFocusRing, and removed its
+        unused style argument.
+
 2011-02-01  Patrick Gansterer  <paroga at webkit.org>
 
         Unreviewed WinCE build fix for r77286.
diff --git a/Source/WebCore/html/HTMLAreaElement.cpp b/Source/WebCore/html/HTMLAreaElement.cpp
index ac4c865..5b0a028 100644
--- a/Source/WebCore/html/HTMLAreaElement.cpp
+++ b/Source/WebCore/html/HTMLAreaElement.cpp
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 1999 Lars Knoll (knoll at kde.org)
  *           (C) 1999 Antti Koivisto (koivisto at kde.org)
- * Copyright (C) 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2009, 2011 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -30,7 +30,7 @@
 #include "HTMLNames.h"
 #include "HitTestResult.h"
 #include "Path.h"
-#include "RenderObject.h"
+#include "RenderImage.h"
 
 using namespace std;
 
@@ -196,33 +196,34 @@ bool HTMLAreaElement::isFocusable() const
     return supportsFocus() && Element::tabIndex() >= 0;
 }
     
-void HTMLAreaElement::dispatchBlurEvent()
+void HTMLAreaElement::setFocus(bool shouldBeFocused)
 {
-    HTMLAnchorElement::dispatchBlurEvent();
-    
-    // On a blur, we might need to remove our focus rings by repainting.
-    updateFocusAppearance(false);
+    if (focused() == shouldBeFocused)
+        return;
+
+    HTMLAnchorElement::setFocus(shouldBeFocused);
+
+    HTMLImageElement* imageElement = this->imageElement();
+    if (!imageElement)
+        return;
+
+    RenderObject* renderer = imageElement->renderer();
+    if (!renderer || !renderer->isImage())
+        return;
+
+    toRenderImage(renderer)->areaElementFocusChanged(this);
 }
     
 void HTMLAreaElement::updateFocusAppearance(bool restorePreviousSelection)
 {
     if (!isFocusable())
         return;
-    
-    ContainerNode* parent = parentNode();
-    if (!parent || !parent->hasTagName(mapTag))
-        return;
-    
-    HTMLImageElement* imageElement = static_cast<HTMLMapElement*>(parent)->imageElement();
+
+    HTMLImageElement* imageElement = this->imageElement();
     if (!imageElement)
         return;
-    
-    // This will handle scrolling to the image if necessary.
+
     imageElement->updateFocusAppearance(restorePreviousSelection);
-    
-    RenderObject* imageRenderer = imageElement->renderer();
-    if (imageRenderer)
-        imageRenderer->setNeedsLayout(true);
 }
     
 bool HTMLAreaElement::supportsFocus() const
diff --git a/Source/WebCore/html/HTMLAreaElement.h b/Source/WebCore/html/HTMLAreaElement.h
index 42d4198..db172b5 100644
--- a/Source/WebCore/html/HTMLAreaElement.h
+++ b/Source/WebCore/html/HTMLAreaElement.h
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 1999 Lars Knoll (knoll at kde.org)
  *           (C) 1999 Antti Koivisto (koivisto at kde.org)
- * Copyright (C) 2004, 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2008, 2009, 2011 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -57,7 +57,7 @@ private:
     virtual bool isMouseFocusable() const;
     virtual bool isFocusable() const;
     virtual void updateFocusAppearance(bool /*restorePreviousSelection*/);
-    virtual void dispatchBlurEvent();
+    virtual void setFocus(bool);
     
     enum Shape { Default, Poly, Rect, Circle, Unknown };
     Path getRegion(const IntSize&) const;
diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
index 41c8d76..cb5828e 100644
--- a/Source/WebCore/rendering/RenderImage.cpp
+++ b/Source/WebCore/rendering/RenderImage.cpp
@@ -4,7 +4,7 @@
  *           (C) 2000 Dirk Mueller (mueller at kde.org)
  *           (C) 2006 Allan Sandfeld Jensen (kde at carewolf.com)
  *           (C) 2006 Samuel Weinig (sam.weinig at gmail.com)
- * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ -30,7 +30,6 @@
 #include "Frame.h"
 #include "GraphicsContext.h"
 #include "HTMLAreaElement.h"
-#include "HTMLCollection.h"
 #include "HTMLImageElement.h"
 #include "HTMLInputElement.h"
 #include "HTMLMapElement.h"
@@ -38,11 +37,9 @@
 #include "HitTestResult.h"
 #include "Page.h"
 #include "RenderLayer.h"
-#include "RenderTheme.h"
 #include "RenderView.h"
 #include "SelectionController.h"
 #include "TextRun.h"
-#include <wtf/CurrentTime.h>
 #include <wtf/UnusedParam.h>
 
 #if ENABLE(WML)
@@ -324,47 +321,51 @@ void RenderImage::paint(PaintInfo& paintInfo, int tx, int ty)
     RenderReplaced::paint(paintInfo, tx, ty);
     
     if (paintInfo.phase == PaintPhaseOutline)
-        paintFocusRing(paintInfo, style());
+        paintAreaElementFocusRing(paintInfo);
 }
     
-void RenderImage::paintFocusRing(PaintInfo& paintInfo, const RenderStyle*)
+void RenderImage::paintAreaElementFocusRing(PaintInfo& paintInfo)
 {
-    // Don't draw focus rings if printing.
-    if (document()->printing() || !frame()->selection()->isFocusedAndActive())
+    Document* document = this->document();
+    
+    if (document->printing() || !document->frame()->selection()->isFocusedAndActive())
         return;
     
     if (paintInfo.context->paintingDisabled() && !paintInfo.context->updatingControlTints())
         return;
 
-    HTMLMapElement* mapElement = imageMap();
-    if (!mapElement)
+    Node* focusedNode = document->focusedNode();
+    if (!focusedNode || !focusedNode->hasTagName(areaTag))
         return;
-    
-    Document* document = mapElement->document();
-    if (!document)
+
+    HTMLAreaElement* areaElement = static_cast<HTMLAreaElement*>(focusedNode);
+    if (areaElement->imageElement() != node())
         return;
-    
-    Node* focusedNode = document->focusedNode();
-    if (!focusedNode)
+
+    // Even if the theme handles focus ring drawing for entire elements, it won't do it for
+    // an area within an image, so we don't call RenderTheme::supportsFocusRing here.
+
+    Path path = areaElement->getPath(this);
+    if (path.isEmpty())
         return;
-    
-    RefPtr<HTMLCollection> areas = mapElement->areas();
-    unsigned numAreas = areas->length();
-    
-    // FIXME: Clip the paths to the image bounding box.
-    for (unsigned k = 0; k < numAreas; ++k) {
-        HTMLAreaElement* areaElement = static_cast<HTMLAreaElement*>(areas->item(k));
-        if (focusedNode != areaElement)
-            continue;
-
-        RenderStyle* styleToUse = areaElement->computedStyle();
-        if (theme()->supportsFocusRing(styleToUse))
-            return; // The theme draws the focus ring.
-        paintInfo.context->drawFocusRing(areaElement->getPath(this), styleToUse->outlineWidth(), styleToUse->outlineOffset(), styleToUse->visitedDependentColor(CSSPropertyOutlineColor));
-        break;
-    }
+
+    // FIXME: Do we need additional code to clip the path to the image's bounding box?
+
+    RenderStyle* areaElementStyle = areaElement->computedStyle();
+    paintInfo.context->drawFocusRing(path, areaElementStyle->outlineWidth(), areaElementStyle->outlineOffset(),
+        areaElementStyle->visitedDependentColor(CSSPropertyOutlineColor));
 }
-    
+
+void RenderImage::areaElementFocusChanged(HTMLAreaElement* element)
+{
+    ASSERT_UNUSED(element, element->imageElement() == node());
+
+    // It would be more efficient to only repaint the focus ring rectangle
+    // for the passed-in area element. That would require adding functions
+    // to the area element class.
+    repaint();
+}
+
 void RenderImage::paintIntoRect(GraphicsContext* context, const IntRect& rect)
 {
     if (!m_imageResource->hasImage() || m_imageResource->errorOccurred() || rect.width() <= 0 || rect.height() <= 0)
diff --git a/Source/WebCore/rendering/RenderImage.h b/Source/WebCore/rendering/RenderImage.h
index 16ae7ec..e3f743c 100644
--- a/Source/WebCore/rendering/RenderImage.h
+++ b/Source/WebCore/rendering/RenderImage.h
@@ -3,7 +3,7 @@
  *           (C) 1999 Antti Koivisto (koivisto at kde.org)
  *           (C) 2006 Allan Sandfeld Jensen (kde at carewolf.com) 
  *           (C) 2006 Samuel Weinig (sam.weinig at gmail.com)
- * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010, 2011 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -30,6 +30,7 @@
 
 namespace WebCore {
 
+class HTMLAreaElement;
 class HTMLMapElement;
 
 class RenderImage : public RenderReplaced {
@@ -48,6 +49,7 @@ public:
     void updateAltText();
 
     HTMLMapElement* imageMap() const;
+    void areaElementFocusChanged(HTMLAreaElement*);
 
     void highQualityRepaintTimerFired(Timer<RenderImage>*);
 
@@ -57,7 +59,6 @@ protected:
     virtual void imageChanged(WrappedImagePtr, const IntRect* = 0);
 
     virtual void paintIntoRect(GraphicsContext*, const IntRect&);
-    void paintFocusRing(PaintInfo&, const RenderStyle*);
     virtual void paint(PaintInfo&, int tx, int ty);
 
     bool isLogicalWidthSpecified() const;
@@ -91,6 +92,8 @@ private:
     int calcAspectRatioLogicalWidth() const;
     int calcAspectRatioLogicalHeight() const;
 
+    void paintAreaElementFocusRing(PaintInfo&);
+
     // Text to display as long as the image isn't available.
     String m_altText;
     OwnPtr<RenderImageResource> m_imageResource;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list