Commit f152cbdd authored by Ionel Popescu's avatar Ionel Popescu Committed by Commit Bot

Use outline-offset for default focus ring.

Prior to this CL, the outline-offset was *ignored when drawing the
default focus ring (see AdjustedFocusRingOffset):
- for FormControlsRefresh, GetDefaultOffsetForFocusRing was used instead
- on Windows the value was completely ignored
- on Mac the value was used

This CL updated the behavior so that the default focus ring respects the
value of outline-offset.
Because of this, GetDefaultOffsetForFocusRing is not needed anymore
since now we can directly update the outline-offset to correctly
reflect what we are painting.
Another difference in behavior is that the offset is not going to be
zoom independent anymore.

This fix is just for FormControlsRefresh (the feature is enabled
on all platforms except Android) to minimize regression risks.

The fix is validated by: focus-ring-outline-offset.html

Bug: 945181
Change-Id: I2cff82556571d9f541b95f8cece9b7c3be1191c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2138156
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756951}
parent 8e46df33
...@@ -16,6 +16,19 @@ textarea { ...@@ -16,6 +16,19 @@ textarea {
border-color: -internal-light-dark-color(#767676, #C3C3C3); border-color: -internal-light-dark-color(#767676, #C3C3C3);
} }
a:-webkit-any-link:focus {
outline-offset: 1px;
}
input:focus, textarea:focus, select:focus {
outline-offset: 0;
}
input[type="checkbox" i]:focus,
input[type="radio" i]:focus {
outline-offset: 2px;
}
input, input,
input[type="email" i], input[type="email" i],
input[type="number" i], input[type="number" i],
......
...@@ -550,8 +550,7 @@ void ObjectPainterBase::PaintOutlineRects( ...@@ -550,8 +550,7 @@ void ObjectPainterBase::PaintOutlineRects(
float border_radius = GetFocusRingBorderRadius(style); float border_radius = GetFocusRingBorderRadius(style);
paint_info.context.DrawFocusRing( paint_info.context.DrawFocusRing(
pixel_snapped_outline_rects, style.GetOutlineStrokeWidthForFocusRing(), pixel_snapped_outline_rects, style.GetOutlineStrokeWidthForFocusRing(),
style.OutlineOffset(), style.GetDefaultOffsetForFocusRing(), style.OutlineOffset(), border_radius, min_border_width, color);
border_radius, min_border_width, color);
return; return;
} }
......
...@@ -2234,8 +2234,7 @@ int ComputedStyle::OutlineOutsetExtent() const { ...@@ -2234,8 +2234,7 @@ int ComputedStyle::OutlineOutsetExtent() const {
return 0; return 0;
if (OutlineStyleIsAuto()) { if (OutlineStyleIsAuto()) {
return GraphicsContext::FocusRingOutsetExtent( return GraphicsContext::FocusRingOutsetExtent(
OutlineOffset(), GetDefaultOffsetForFocusRing(), OutlineOffset(), std::ceil(GetOutlineStrokeWidthForFocusRing()));
std::ceil(GetOutlineStrokeWidthForFocusRing()));
} }
return base::ClampAdd(OutlineWidth(), OutlineOffset()).Max(0); return base::ClampAdd(OutlineWidth(), OutlineOffset()).Max(0);
} }
...@@ -2254,20 +2253,6 @@ float ComputedStyle::GetOutlineStrokeWidthForFocusRing() const { ...@@ -2254,20 +2253,6 @@ float ComputedStyle::GetOutlineStrokeWidthForFocusRing() const {
#endif #endif
} }
int ComputedStyle::GetDefaultOffsetForFocusRing() const {
if (!::features::IsFormControlsRefreshEnabled())
return 0;
if (EffectiveAppearance() == kCheckboxPart ||
EffectiveAppearance() == kRadioPart) {
return 2;
} else if (IsLink()) {
return 1;
}
return 0;
}
bool ComputedStyle::ColumnRuleEquivalent( bool ComputedStyle::ColumnRuleEquivalent(
const ComputedStyle& other_style) const { const ComputedStyle& other_style) const {
return ColumnRuleStyle() == other_style.ColumnRuleStyle() && return ColumnRuleStyle() == other_style.ColumnRuleStyle() &&
......
...@@ -2006,7 +2006,6 @@ class ComputedStyle : public ComputedStyleBase, ...@@ -2006,7 +2006,6 @@ class ComputedStyle : public ComputedStyleBase,
} }
CORE_EXPORT int OutlineOutsetExtent() const; CORE_EXPORT int OutlineOutsetExtent() const;
CORE_EXPORT float GetOutlineStrokeWidthForFocusRing() const; CORE_EXPORT float GetOutlineStrokeWidthForFocusRing() const;
CORE_EXPORT int GetDefaultOffsetForFocusRing() const;
bool HasOutlineWithCurrentColor() const { bool HasOutlineWithCurrentColor() const {
return HasOutline() && OutlineColor().IsCurrentColor(); return HasOutline() && OutlineColor().IsCurrentColor();
} }
......
...@@ -105,8 +105,6 @@ TEST(ComputedStyleTest, FocusRingOutset) { ...@@ -105,8 +105,6 @@ TEST(ComputedStyleTest, FocusRingOutset) {
style->SetEffectiveZoom(4.75); style->SetEffectiveZoom(4.75);
if (::features::IsFormControlsRefreshEnabled()) { if (::features::IsFormControlsRefreshEnabled()) {
EXPECT_EQ(4, style->OutlineOutsetExtent()); EXPECT_EQ(4, style->OutlineOutsetExtent());
style->SetEffectiveAppearance(kRadioPart);
EXPECT_EQ(6, style->OutlineOutsetExtent());
} else { } else {
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
EXPECT_EQ(4, style->OutlineOutsetExtent()); EXPECT_EQ(4, style->OutlineOutsetExtent());
......
...@@ -322,11 +322,10 @@ void GraphicsContext::CompositeRecord(sk_sp<PaintRecord> record, ...@@ -322,11 +322,10 @@ void GraphicsContext::CompositeRecord(sk_sp<PaintRecord> record,
namespace { namespace {
int AdjustedFocusRingOffset(int offset, int default_offset, int width) { int AdjustedFocusRingOffset(int offset, int width) {
if (::features::IsFormControlsRefreshEnabled()) { if (::features::IsFormControlsRefreshEnabled()) {
// For FormControlsRefresh the focus ring has a default offset that // For FormControlsRefresh just use the value of outline-offset.
// depends on the element type. return offset;
return default_offset;
} }
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -339,19 +338,16 @@ int AdjustedFocusRingOffset(int offset, int default_offset, int width) { ...@@ -339,19 +338,16 @@ int AdjustedFocusRingOffset(int offset, int default_offset, int width) {
} // namespace } // namespace
int GraphicsContext::FocusRingOutsetExtent(int offset, int GraphicsContext::FocusRingOutsetExtent(int offset,
int default_offset,
int width) { int width) {
// Unlike normal outlines (whole width is outside of the offset), focus // Unlike normal outlines (whole width is outside of the offset), focus
// rings can be drawn with the center of the path aligned with the offset, so // rings can be drawn with the center of the path aligned with the offset, so
// only half of the width is outside of the offset. // only half of the width is outside of the offset.
if (::features::IsFormControlsRefreshEnabled()) { if (::features::IsFormControlsRefreshEnabled()) {
// For FormControlsRefresh 2/3 of the width is outside of the offset. // For FormControlsRefresh 2/3 of the width is outside of the offset.
return AdjustedFocusRingOffset(offset, default_offset, width) + return AdjustedFocusRingOffset(offset, width) + std::ceil(width / 3.f) * 2;
std::ceil(width / 3.f) * 2;
} }
return AdjustedFocusRingOffset(offset, /*default_offset=*/0, width) + return AdjustedFocusRingOffset(offset, width) + (width + 1) / 2;
(width + 1) / 2;
} }
void GraphicsContext::DrawFocusRingPath(const SkPath& path, void GraphicsContext::DrawFocusRingPath(const SkPath& path,
...@@ -400,8 +396,7 @@ void GraphicsContext::DrawFocusRingInternal(const Vector<IntRect>& rects, ...@@ -400,8 +396,7 @@ void GraphicsContext::DrawFocusRingInternal(const Vector<IntRect>& rects,
if (!::features::IsFormControlsRefreshEnabled()) { if (!::features::IsFormControlsRefreshEnabled()) {
// For FormControlsRefresh the offset is already adjusted by // For FormControlsRefresh the offset is already adjusted by
// GraphicsContext::DrawFocusRing. // GraphicsContext::DrawFocusRing.
offset = offset = AdjustedFocusRingOffset(offset, std::ceil(width));
AdjustedFocusRingOffset(offset, /*default_offset=*/0, std::ceil(width));
} }
for (unsigned i = 0; i < rect_count; i++) { for (unsigned i = 0; i < rect_count; i++) {
SkIRect r = rects[i]; SkIRect r = rects[i];
...@@ -427,7 +422,6 @@ void GraphicsContext::DrawFocusRingInternal(const Vector<IntRect>& rects, ...@@ -427,7 +422,6 @@ void GraphicsContext::DrawFocusRingInternal(const Vector<IntRect>& rects,
void GraphicsContext::DrawFocusRing(const Vector<IntRect>& rects, void GraphicsContext::DrawFocusRing(const Vector<IntRect>& rects,
float width, float width,
int offset, int offset,
int default_offset,
float border_radius, float border_radius,
float min_border_width, float min_border_width,
const Color& color) { const Color& color) {
...@@ -436,7 +430,7 @@ void GraphicsContext::DrawFocusRing(const Vector<IntRect>& rects, ...@@ -436,7 +430,7 @@ void GraphicsContext::DrawFocusRing(const Vector<IntRect>& rects,
const float first_border_width = (width / 3) * 2; const float first_border_width = (width / 3) * 2;
const float second_border_width = width - first_border_width; const float second_border_width = width - first_border_width;
offset = AdjustedFocusRingOffset(offset, default_offset, std::ceil(width)); offset = AdjustedFocusRingOffset(offset, std::ceil(width));
// How much space the focus ring would like to take from the actual border. // How much space the focus ring would like to take from the actual border.
const float inside_border_width = 1; const float inside_border_width = 1;
if (min_border_width >= inside_border_width) { if (min_border_width >= inside_border_width) {
......
...@@ -362,7 +362,6 @@ class PLATFORM_EXPORT GraphicsContext { ...@@ -362,7 +362,6 @@ class PLATFORM_EXPORT GraphicsContext {
void DrawFocusRing(const Vector<IntRect>&, void DrawFocusRing(const Vector<IntRect>&,
float width, float width,
int offset, int offset,
int default_offset,
float border_radius, float border_radius,
float min_border_width, float min_border_width,
const Color&); const Color&);
...@@ -421,7 +420,7 @@ class PLATFORM_EXPORT GraphicsContext { ...@@ -421,7 +420,7 @@ class PLATFORM_EXPORT GraphicsContext {
FloatPoint& p2, FloatPoint& p2,
float stroke_width); float stroke_width);
static int FocusRingOutsetExtent(int offset, int default_offset, int width); static int FocusRingOutsetExtent(int offset, int width);
void SetInDrawingRecorder(bool); void SetInDrawingRecorder(bool);
bool InDrawingRecorder() const { return in_drawing_recorder_; } bool InDrawingRecorder() const { return in_drawing_recorder_; }
......
...@@ -129,3 +129,7 @@ crbug.com/1041322 virtual/threaded/synthetic_gestures/synthetic-pinch-zoom-gestu ...@@ -129,3 +129,7 @@ crbug.com/1041322 virtual/threaded/synthetic_gestures/synthetic-pinch-zoom-gestu
compositing/gestures/gesture-tapHighlight-composited-img.html [ Pass Failure ] compositing/gestures/gesture-tapHighlight-composited-img.html [ Pass Failure ]
http/tests/images/image-decode-in-frame.html [ Pass Failure ] http/tests/images/image-decode-in-frame.html [ Pass Failure ]
# TODO(iopopesc) these need to be rebaselined for FormControlsRefresh focus ring.
crbug.com/1035582 virtual/scalefactor200withzoom/fast/hidpi/static/validation-bubble-appearance-hidpi.html [ Failure ]
crbug.com/1035582 compositing/overflow/do-not-paint-outline-into-composited-scrolling-contents.html [ Failure ]
...@@ -2144,6 +2144,7 @@ crbug.com/953591 [ Win ] virtual/form-controls-refresh-disabled/fast/forms/datal ...@@ -2144,6 +2144,7 @@ crbug.com/953591 [ Win ] virtual/form-controls-refresh-disabled/fast/forms/datal
crbug.com/591099 [ Mac ] virtual/form-controls-refresh-disabled/fast/forms/text/text-lineheight-centering.html [ Failure ] crbug.com/591099 [ Mac ] virtual/form-controls-refresh-disabled/fast/forms/text/text-lineheight-centering.html [ Failure ]
crbug.com/770971 [ Win7 ] virtual/form-controls-refresh-disabled/fast/forms/suggested-value.html [ Pass Failure ] crbug.com/770971 [ Win7 ] virtual/form-controls-refresh-disabled/fast/forms/suggested-value.html [ Pass Failure ]
crbug.com/1035582 virtual/form-controls-refresh-disabled/fast/forms/number/number-input-event-composed.html [ Pass Failure ] crbug.com/1035582 virtual/form-controls-refresh-disabled/fast/forms/number/number-input-event-composed.html [ Pass Failure ]
crbug.com/1035582 [ Mac ] virtual/form-controls-refresh-disabled/fast/forms/text/input-readonly-focus-ring.html [ Skip ]
# From never fix tests: # From never fix tests:
crbug.com/123456 [ Mac ] virtual/form-controls-refresh-disabled/fast/forms/select-popup/* [ Skip ] crbug.com/123456 [ Mac ] virtual/form-controls-refresh-disabled/fast/forms/select-popup/* [ Skip ]
......
<input id="target" readonly style="outline-offset: -2px; outline: -webkit-focus-ring-color auto 5px;"> <input id="target" readonly style="outline-offset: 0px; outline: -webkit-focus-ring-color auto 5px;">
<!DOCTYPE html>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<body style="background-color: red">
<!-- no style for reference -->
<input type="button" id="focusTarget" value="button" />
<br>
<br>
<input type="checkbox" checked id="focusTarget" />
<br>
<br>
<input type="radio" id="focusTarget" />
<br>
<br>
<input type="text" value="example text" id="focusTarget" />
<br>
<br>
<a href="#" id="focusTarget" >Test</a>
<br>
<br>
<!-- outline-offset: 4px -->
<input type="button" style="outline-offset: 4px" value="button" id="focusTarget" />
<br>
<br>
<input type="checkbox" style="outline-offset: 4px" checked id="focusTarget" />
<br>
<br>
<input type="radio" style="outline-offset: 4px" id="focusTarget" />
<br>
<br>
<input type="text" style="outline-offset: 4px" value="example text" id="focusTarget" />
<br>
<br>
<a href="#" style="outline-offset: 4px" id="focusTarget">Test</a>
<br>
<script>
runAfterLayoutAndPaint(function() {
document.querySelectorAll("#focusTarget").forEach(node => {
internals.setPseudoClassState(node, ":focus", true);
// console.log(node);
});
}, true);
</script>
</body>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment