Changeset 77313 in webkit


Ignore:
Timestamp:
Feb 1, 2011 2:49:01 PM (13 years ago)
Author:
Darin Adler
Message:

2011-02-01 Darin Adler <Darin Adler>

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 Adler>

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.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r77311 r77313  
     12011-02-01  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Chris Fleizach.
     4
     5        REGRESSION: Removing focus from area element causes unwanted scrolling
     6        https://bugs.webkit.org/show_bug.cgi?id=50169
     7
     8        * fast/images/imagemap-scroll-expected.txt: Added.
     9        * fast/images/imagemap-scroll.html: Added.
     10
    1112011-02-01  Zhenyao Mo  <zmo@google.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r77311 r77313  
     12011-02-01  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Chris Fleizach.
     4
     5        REGRESSION: Removing focus from area element causes unwanted scrolling
     6        https://bugs.webkit.org/show_bug.cgi?id=50169
     7
     8        Test: fast/images/imagemap-scroll.html
     9
     10        * html/HTMLAreaElement.cpp:
     11        (WebCore::HTMLAreaElement::setFocus): Added override. Calls the new
     12        RenderImage::areaElementFocusChanged function.
     13        (WebCore::HTMLAreaElement::updateFocusAppearance): Removed the code
     14        here that calls setNeedsLayout on the image's renderer. This was an
     15        attempt to cause repaint of the renderer, but this function does not
     16        need to do that. Also changed this to use the imageElement function
     17        to avoid repeating code.
     18
     19        * html/HTMLAreaElement.h: Updated for above changes.
     20
     21        * rendering/RenderImage.cpp:
     22        (WebCore::RenderImage::paint): Updated for name change.
     23        (WebCore::RenderImage::paintAreaElementFocusRing): Renamed this from
     24        paintFocusRing, because it only paints area focus rings, and should
     25        not be confused with paintFocusRing functions in other classes. Also
     26        removed the unused style argument. Removed the code that used an
     27        HTMLCollection to see if the focused area element is for this image
     28        and instead just call imageElement on the area element.
     29        (WebCore::RenderImage::areaElementFocusChanged): Added. Calls repaint.
     30
     31        * rendering/RenderImage.h: Added a public areaElementFocusChanged
     32        function for HTMLAreaElement to call. Made the paintFocusRing function
     33        private, renamed it to paintAreaElementFocusRing, and removed its
     34        unused style argument.
     35
    1362011-02-01  Patrick Gansterer  <paroga@webkit.org>
    237

  • trunk/Source/WebCore/html/HTMLAreaElement.cpp

    r76170 r77313  
    22 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
    33 *           (C) 1999 Antti Koivisto (koivisto@kde.org)
    4  * Copyright (C) 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
     4 * Copyright (C) 2004, 2005, 2006, 2009, 2011 Apple Inc. All rights reserved.
    55 *
    66 * This library is free software; you can redistribute it and/or
    … …  
    3131#include "HitTestResult.h"
    3232#include "Path.h"
    33 #include "RenderObject.h"
     33#include "RenderImage.h"
    3434
    3535using namespace std;
    … …  
    197197}
    198198   
    199 void HTMLAreaElement::dispatchBlurEvent()
    200 {
    201     HTMLAnchorElement::dispatchBlurEvent();
    202    
    203     // On a blur, we might need to remove our focus rings by repainting.
    204     updateFocusAppearance(false);
     199void HTMLAreaElement::setFocus(bool shouldBeFocused)
     200{
     201    if (focused() == shouldBeFocused)
     202        return;
     203
     204    HTMLAnchorElement::setFocus(shouldBeFocused);
     205
     206    HTMLImageElement* imageElement = this->imageElement();
     207    if (!imageElement)
     208        return;
     209
     210    RenderObject* renderer = imageElement->renderer();
     211    if (!renderer || !renderer->isImage())
     212        return;
     213
     214    toRenderImage(renderer)->areaElementFocusChanged(this);
    205215}
    206216   
    … …  
    209219    if (!isFocusable())
    210220        return;
    211    
    212     ContainerNode* parent = parentNode();
    213     if (!parent || !parent->hasTagName(mapTag))
    214         return;
    215    
    216     HTMLImageElement* imageElement = static_cast<HTMLMapElement*>(parent)->imageElement();
     221
     222    HTMLImageElement* imageElement = this->imageElement();
    217223    if (!imageElement)
    218224        return;
    219    
    220     // This will handle scrolling to the image if necessary.
     225
    221226    imageElement->updateFocusAppearance(restorePreviousSelection);
    222    
    223     RenderObject* imageRenderer = imageElement->renderer();
    224     if (imageRenderer)
    225         imageRenderer->setNeedsLayout(true);
    226227}
    227228   

  • trunk/Source/WebCore/html/HTMLAreaElement.h

    r69237 r77313  
    22 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
    33 *           (C) 1999 Antti Koivisto (koivisto@kde.org)
    4  * Copyright (C) 2004, 2008, 2009 Apple Inc. All rights reserved.
     4 * Copyright (C) 2004, 2008, 2009, 2011 Apple Inc. All rights reserved.
    55 *
    66 * This library is free software; you can redistribute it and/or
    … …  
    5858    virtual bool isFocusable() const;
    5959    virtual void updateFocusAppearance(bool /*restorePreviousSelection*/);
    60     virtual void dispatchBlurEvent();
     60    virtual void setFocus(bool);
    6161   
    6262    enum Shape { Default, Poly, Rect, Circle, Unknown };

  • trunk/Source/WebCore/rendering/RenderImage.cpp

    r76866 r77313  
    55 *           (C) 2006 Allan Sandfeld Jensen (kde@carewolf.com)
    66 *           (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    7  * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
     7 * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
    88 * Copyright (C) 2010 Google Inc. All rights reserved.
    99 *
    … …  
    3131#include "GraphicsContext.h"
    3232#include "HTMLAreaElement.h"
    33 #include "HTMLCollection.h"
    3433#include "HTMLImageElement.h"
    3534#include "HTMLInputElement.h"
    … …  
    3938#include "Page.h"
    4039#include "RenderLayer.h"
    41 #include "RenderTheme.h"
    4240#include "RenderView.h"
    4341#include "SelectionController.h"
    4442#include "TextRun.h"
    45 #include <wtf/CurrentTime.h>
    4643#include <wtf/UnusedParam.h>
    4744
    … …  
    325322   
    326323    if (paintInfo.phase == PaintPhaseOutline)
    327         paintFocusRing(paintInfo, style());
     324        paintAreaElementFocusRing(paintInfo);
    328325}
    329326   
    330 void RenderImage::paintFocusRing(PaintInfo& paintInfo, const RenderStyle*)
    331 {
    332     // Don't draw focus rings if printing.
    333     if (document()->printing() || !frame()->selection()->isFocusedAndActive())
     327void RenderImage::paintAreaElementFocusRing(PaintInfo& paintInfo)
     328{
     329    Document* document = this->document();
     330   
     331    if (document->printing() || !document->frame()->selection()->isFocusedAndActive())
    334332        return;
    335333   
    … …  
    337335        return;
    338336
    339     HTMLMapElement* mapElement = imageMap();
    340     if (!mapElement)
    341         return;
    342    
    343     Document* document = mapElement->document();
    344     if (!document)
    345         return;
    346    
    347337    Node* focusedNode = document->focusedNode();
    348     if (!focusedNode)
    349         return;
    350    
    351     RefPtr<HTMLCollection> areas = mapElement->areas();
    352     unsigned numAreas = areas->length();
    353    
    354     // FIXME: Clip the paths to the image bounding box.
    355     for (unsigned k = 0; k < numAreas; ++k) {
    356         HTMLAreaElement* areaElement = static_cast<HTMLAreaElement*>(areas->item(k));
    357         if (focusedNode != areaElement)
    358             continue;
    359 
    360         RenderStyle* styleToUse = areaElement->computedStyle();
    361         if (theme()->supportsFocusRing(styleToUse))
    362             return; // The theme draws the focus ring.
    363         paintInfo.context->drawFocusRing(areaElement->getPath(this), styleToUse->outlineWidth(), styleToUse->outlineOffset(), styleToUse->visitedDependentColor(CSSPropertyOutlineColor));
    364         break;
    365     }
    366 }
    367    
     338    if (!focusedNode || !focusedNode->hasTagName(areaTag))
     339        return;
     340
     341    HTMLAreaElement* areaElement = static_cast<HTMLAreaElement*>(focusedNode);
     342    if (areaElement->imageElement() != node())
     343        return;
     344
     345    // Even if the theme handles focus ring drawing for entire elements, it won't do it for
     346    // an area within an image, so we don't call RenderTheme::supportsFocusRing here.
     347
     348    Path path = areaElement->getPath(this);
     349    if (path.isEmpty())
     350        return;
     351
     352    // FIXME: Do we need additional code to clip the path to the image's bounding box?
     353
     354    RenderStyle* areaElementStyle = areaElement->computedStyle();
     355    paintInfo.context->drawFocusRing(path, areaElementStyle->outlineWidth(), areaElementStyle->outlineOffset(),
     356        areaElementStyle->visitedDependentColor(CSSPropertyOutlineColor));
     357}
     358
     359void RenderImage::areaElementFocusChanged(HTMLAreaElement* element)
     360{
     361    ASSERT_UNUSED(element, element->imageElement() == node());
     362
     363    // It would be more efficient to only repaint the focus ring rectangle
     364    // for the passed-in area element. That would require adding functions
     365    // to the area element class.
     366    repaint();
     367}
     368
    368369void RenderImage::paintIntoRect(GraphicsContext* context, const IntRect& rect)
    369370{

  • trunk/Source/WebCore/rendering/RenderImage.h

    r72528 r77313  
    44 *           (C) 2006 Allan Sandfeld Jensen (kde@carewolf.com)
    55 *           (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    6  * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010 Apple Inc. All rights reserved.
     6 * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010, 2011 Apple Inc. All rights reserved.
    77 *
    88 * This library is free software; you can redistribute it and/or
    … …  
    3131namespace WebCore {
    3232
     33class HTMLAreaElement;
    3334class HTMLMapElement;
    3435
    … …  
    4950
    5051    HTMLMapElement* imageMap() const;
     52    void areaElementFocusChanged(HTMLAreaElement*);
    5153
    5254    void highQualityRepaintTimerFired(Timer<RenderImage>*);
    … …  
    5860
    5961    virtual void paintIntoRect(GraphicsContext*, const IntRect&);
    60     void paintFocusRing(PaintInfo&, const RenderStyle*);
    6162    virtual void paint(PaintInfo&, int tx, int ty);
    6263
    … …  
    9293    int calcAspectRatioLogicalHeight() const;
    9394
     95    void paintAreaElementFocusRing(PaintInfo&);
     96
    9497    // Text to display as long as the image isn't available.
    9598    String m_altText;
Note: See TracChangeset for help on using the changeset viewer.