Make anchors mouse-focusable

Source/core/html/HTMLAnchorElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Simon Hausmann <hausmann@kde.org>
  * Copyright (C) 2003, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 25 matching lines @@
 #include "core/html/HTMLFormElement.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/loader/FrameLoadRequest.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/loader/FrameLoaderTypes.h"
 #include "core/loader/PingLoader.h"
 #include "core/page/Chrome.h"
 #include "core/page/ChromeClient.h"
+#include "core/page/FocusController.h"
+#include "core/page/Page.h"
 #include "core/rendering/RenderImage.h"
 #include "platform/PlatformMouseEvent.h"
 #include "platform/network/DNS.h"
 #include "platform/network/ResourceRequest.h"
 #include "platform/weborigin/KnownPorts.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "platform/weborigin/SecurityPolicy.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebURL.h"
 #include "public/platform/WebURLRequest.h"
@@ skipping 21 matching lines @@
 }
 
 bool HTMLAnchorElement::supportsFocus() const
 {
     if (hasEditableStyle())
         return HTMLElement::supportsFocus();
     // If not a link we should still be able to focus the element if it has tabIndex.
     return isLink() || HTMLElement::supportsFocus();
 }
 
+bool HTMLAnchorElement::shouldHaveFocusAppearance() const
+{
+    ASSERT(focused());
+    return !document().page()->focusController().wasFocusedByMouse() || HTMLElement::supportsFocus();

[Xianzhu, 2014/08/12 03:58:01]

The above code will have side-effect. For example:

<a href="#" style="outline:red auto 5px">...</a>
<a href="#">...</a>

When the second element is focused by mouse, the first element will lose its
outline because of the global wasFocusedByMouse status becomes false. Depending
on whether the tile(s) containing the first element is invalidated when the
second element is repainted, the first element will appear still having outline
(unstable), not having outline, or corrupted.

The flag introduces another factor besides DOM structure and styles affecting
element appearance, which might be agile. Can we let it trigger a style change?
(e.g. sth like :focus[_bymouse] { outline: none })

[Xianzhu, 2014/08/12 04:01:11]

s/agile/fragile/ ..

[robwu, 2014/08/12 14:27:49]

What is "outline-style:auto" supposed to do when an element has no focus?
The spec (CSS3-UI section 7.4) allows "auto" to be treated as "solid".
Currently (before my changes), setting `outline:auto` causes the default focus
ring to be displayed (note: this style does *not* look like "solid"),
irrespective of whether the element is actually focusable. Further outline-width
and outline-offset have no influence on the rendering of the special focus ring
(reported as crbug.com/402874),
Are you suggesting to introduce a new pseudo-class, e.g. :-webkit-mouse-focus,
as in:

/* default style = outline-style:none; */
/* UA stylesheet: */
textarea:focus:not(:-webkit-mouse-focus),
input:focus:not(:-webkit-mouse-focus) {
:link:focus:not(:-webkit-mouse-focus) {
    outline-style: auto;
}
/*  (perhaps -webkit-focus-ring-style instead of "auto" ?) */

[Xianzhu, 2014/08/12 16:03:07]

I think 'outline-style:auto' was first introduced in WebKit just for focus
rings, so the only appearance of it focus ring for now. However, as now it has
been a property value published in spec, we should support it formally.
Corrupted rendering caused by unstable repainting (that is, inconsistent
invalidation and repaint causing for example half outline around an element) is
not acceptable.

We are on the way to remove paintInvalidationForWholeRenderer because it breaks
the principle of repaint-after-layout (now actually 'invalidate at the end of
each frame). We really don't want sources other than style and layout triggering
repaint.
Yes, something like that, to ensure the change of appearance of elements only
controlled by dom structure and css. Your proposal is much better than my
example :)

[Xianzhu, 2014/08/14 19:12:49]

I no longer insist the pseudo-class method. The current method also looks good.
:)

About paintInvalidationForWholeRenderer(), you can change it to
setShouldDoFullPaintInvalidation(true) after
https://codereview.chromium.org/478433002/.

[robwu, 2014/08/15 22:51:33]

Great. Anything else besides that?

+}
+
 bool HTMLAnchorElement::isMouseFocusable() const
 {
-    // Links are focusable by default, but only allow links with tabindex or contenteditable to be mouse focusable.

[pdr., 2014/08/12 02:02:22]

Both here and in SVGAElement we're removing this comment about contenteditable
elements being mouse focusable.

Can you add a test for content editable svg and html links that are selected and
not selected by the mouse?

[robwu, 2014/08/12 14:27:49]

LayoutTests/fast/events/click-focus-anchor.html already has tests for
contentEditable on HTML <a> elements. Do you want similar tests for SVG <a>
elements as well, or did you mean something else?

-    // https://bugs.webkit.org/show_bug.cgi?id=26856
     if (isLink())
-        return HTMLElement::supportsFocus();
+        return supportsFocus();
 
     return HTMLElement::isMouseFocusable();
 }
 
 bool HTMLAnchorElement::isKeyboardFocusable() const
 {
     ASSERT(document().isActive());
 
     if (isFocusable() && Element::supportsFocus())
         return HTMLElement::isKeyboardFocusable();
@@ skipping 277 matching lines @@
             Vector<String> argv;
             argv.append("a");
             argv.append(fastGetAttribute(hrefAttr));
             activityLogger->logEvent("blinkAddElement", argv.size(), argv.data());
         }
     }
     return HTMLElement::insertedInto(insertionPoint);
 }
 
 }