Skip to content

GitLab

T
tangled
Commit 0151427d authored by tkent's avatar tkent Committed by Commit bot
Browse files
Options

SELECT element: Fix a bug that intrinsic width is too narrow in less-than-100% zoom level. 

This is a regression by crbug.com/432795.  Scrollbar thickness is fixed regardless
of zoom level.  So popup width was too narrow in less-than-100% zoom level.

With this CL, popupInternalPaddingEnd() returns zoomed value as ever if zoom level
is 100%+, and returns a value based on actual scrollbar thickness otherwise.

Also, popupIntenalPaddingEnd() respects to the actual scrollbar thickness instead
of returning fixed '18' pixel.  The default scrollbar thickness on Windows is 17.
popupInternalPaddingEnd() returns 1 + <scrollbar thickness>.

We need to update ThemePainterDefault::setupMenuListArrow() so that it can support
variable width of scrollbars.

Summary of behavior changes:

All platforms except Mac:
  menulist box is wider in less-than-100% zoom level.

Windows Aura theme:
  menulist box is not changed in 100%+ zoom level.

Non-Windows Aura theme and Mock theme:
  menulist box  is narrower by 2px because scrollbar thickness is 15px.

BUG=667236
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2560733002
Cr-Commit-Position: refs/heads/master@{#437864}
parent e491471a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 214 additions and 89 deletions
third_party/WebKit/LayoutTests/accessibility/disabled-controls-not-focusable.html
View file @ 0151427d
......@@ -3,7 +3,7 @@
<body>
<script src="../resources/js-test.js"></script>
<div>
<div id="container">
<button id="button"></button>
<input id="text" type="text">
<input id="checkbox" type="checkbox">
......@@ -41,6 +41,8 @@ if (window.testRunner && window.accessibilityController) {
checkControl("combobox");
checkControl("listbox");
checkControl("textarea");
document.querySelector("#container").remove();
}
</script>
......
third_party/WebKit/LayoutTests/fast/canvas/fallback-content.html
View file @ 0151427d
......@@ -2,7 +2,7 @@
<html>
<body>
<script src="../../resources/js-test.js"></script>
<div id="container">
<div>
<a id="link1" href="#">Link</a>
<button id="button1">Button</button>
......@@ -34,7 +34,7 @@
<canvas hidden id="hiddenCanvas" width="300" height="300">
<a id="linkInHiddenCanvas" href="#">Link</a>
</canvas>
</div>
<div id="console"></div>
<script>
description("This test makes sure that focusable elements in canvas fallback content are focusable.");
......@@ -87,6 +87,7 @@ function checkNotFocusable(id) {
checkNotFocusable("linkInHiddenCanvas");
document.querySelector("#container").remove();
</script>
</body>
......
third_party/WebKit/LayoutTests/fast/forms/form-radio-img-node-list-expected.txt
View file @ 0151427d
......@@ -19,5 +19,3 @@ PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/forms/form-radio-img-node-list.html
View file @ 0151427d
<!doctype html>
<script src="../../resources/js-test.js"></script>
<div id="container">
<img name=outside1></img>
<form id="f1">
<button id=n1></button>
......@@ -17,6 +18,7 @@
</form>
<!-- The img element isn't 'reassociatable'; add @form to verify it is so. -->
<img name=n2 form=f1></img>
</div>
<script>
description("Test RadioNodeLists returned by the HTMLFormElement named-getter.");
......@@ -47,4 +49,6 @@ var img = document.createElement("img");
img.name = "n2";
form.appendChild(img);
verifyLength(3);
document.querySelector("#container").remove();
</script>
third_party/WebKit/LayoutTests/fast/forms/select/menulist-appearance-basic.html
View file @ 0151427d
......@@ -51,7 +51,9 @@ if (window.testRunner)
<!-- zoom -->
<select style="zoom: 1.5;"><option>foo</option></select>
<select style="zoom: 2;"><option>foo</option></select> <br>
<select style="zoom: 2;"><option>foo</option></select>
<select style="zoom: 0.5;"><option>foo</option></select>
<br>
<!-- multiple - on platforms that use menulist rendering for <select multiple> tags -->
<select multiple="multiple" style="width: 200px; height: 25px;">
......
third_party/WebKit/LayoutTests/platform/linux/fast/forms/form-radio-img-node-list-expected.txt deleted 100644 → 0
View file @ e491471a
CONSOLE WARNING: The <keygen> element is deprecated and will be removed in M57, around March 2017. See https://www.chromestatus.com/features/5716060992962560 for more details.
Test RadioNodeLists returned by the HTMLFormElement named-getter.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Check that if no 'listed elements' match by name, img elements are picked instead.
PASS radioNodeList.length is 2
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList.length is 2
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList.length is 3
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList[2] instanceof HTMLImageElement is true
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/platform/win/fast/forms/form-radio-img-node-list-expected.txt deleted 100644 → 0
View file @ e491471a
CONSOLE WARNING: The <keygen> element is deprecated and will be removed in M57, around March 2017. See https://www.chromestatus.com/features/5716060992962560 for more details.
Test RadioNodeLists returned by the HTMLFormElement named-getter.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Check that if no 'listed elements' match by name, img elements are picked instead.
PASS radioNodeList.length is 2
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList.length is 2
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList.length is 3
PASS radioNodeList[0] instanceof HTMLImageElement is true
PASS radioNodeList[1] instanceof HTMLImageElement is true
PASS radioNodeList[2] instanceof HTMLImageElement is true
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/platform/win/fast/forms/select/menulist-appearance-basic-expected.txt
View file @ 0151427d
......@@ -106,7 +106,13 @@ layer at (0,0) size 800x523
text run at (8,2) width 38: "foo"
LayoutText {#text} at (181,235) size 4x17
text run at (181,235) width 4: " "
LayoutBR {BR} at (185,235) size 0x17
LayoutMenuList {SELECT} at (187,239) size 35x14 [bgcolor=#FFFFFF] [border: (1px solid #A9A9A9)]
LayoutBlockFlow (anonymous) at (1,1) size 33x12
LayoutText (anonymous) at (2,0) size 14x12
text run at (2,0) width 14: "foo"
LayoutText {#text} at (224,235) size 4x17
text run at (224,235) width 4: " "
LayoutBR {BR} at (228,235) size 0x17
LayoutText {#text} at (208,283) size 4x17
text run at (208,283) width 4: " "
LayoutBR {BR} at (212,283) size 0x17
......
third_party/WebKit/LayoutTests/platform/win/fast/forms/select/menulist-appearance-rtl-expected.txt
View file @ 0151427d
......@@ -62,8 +62,8 @@ layer at (0,0) size 800x540
LayoutText {#text} at (0,0) size 101x16
text run at (0,0) width 101 RTL: "\x{627}\x{644}\x{627}\x{642}\x{62A}\x{631}\x{627}\x{62D}\x{627}\x{62A} / \x{627}\x{644}\x{634}\x{643}\x{627}\x{648}\x{64A}"
LayoutBlockFlow (anonymous) at (0,34) size 784x20
LayoutMenuList {SELECT} at (0,0) size 115x20 [bgcolor=#FFFFFF] [border: (1px solid #A9A9A9)]
LayoutBlockFlow (anonymous) at (1,1) size 113x18
LayoutMenuList {SELECT} at (0,0) size 113x20 [bgcolor=#FFFFFF] [border: (1px solid #A9A9A9)]
LayoutBlockFlow (anonymous) at (1,1) size 111x18
LayoutText (anonymous) at (4,1) size 82x16
text run at (4,1) width 82 RTL: "\x{627}\x{644}\x{627}\x{642}\x{62A}\x{631}\x{627}\x{62D}\x{627}\x{62A} / \x{627}\x{644}\x{634}\x{643}\x{627}\x{648}\x{64A}"
LayoutText {#text} at (0,0) size 0x0
......
third_party/WebKit/Source/core/layout/LayoutMenuList.cpp
View file @ 0151427d
......@@ -28,6 +28,7 @@
#include "core/dom/AXObjectCache.h"
#include "core/dom/NodeComputedStyle.h"
#include "core/frame/FrameView.h"
#include "core/html/HTMLOptionElement.h"
#include "core/html/HTMLSelectElement.h"
#include "core/layout/LayoutText.h"
......@@ -98,8 +99,9 @@ void LayoutMenuList::adjustInnerStyle() {
Length paddingStart =
Length(LayoutTheme::theme().popupInternalPaddingStart(styleRef()), Fixed);
Length paddingEnd =
Length(LayoutTheme::theme().popupInternalPaddingEnd(styleRef()), Fixed);
Length paddingEnd = Length(LayoutTheme::theme().popupInternalPaddingEnd(
frameView()->getHostWindow(), styleRef()),
Fixed);
innerStyle.setPaddingLeft(styleRef().direction() == LTR ? paddingStart
: paddingEnd);
innerStyle.setPaddingRight(styleRef().direction() == LTR ? paddingEnd
......
third_party/WebKit/Source/core/layout/LayoutTheme.h
View file @ 0151427d
......@@ -41,6 +41,7 @@ namespace blink {
class ComputedStyle;
class Element;
class FileList;
class HostWindow;
class HTMLInputElement;
class LayoutObject;
class Theme;
......@@ -167,7 +168,10 @@ class CORE_EXPORT LayoutTheme : public RefCounted<LayoutTheme> {
virtual int popupInternalPaddingStart(const ComputedStyle&) const {
return 0;
}
virtual int popupInternalPaddingEnd(const ComputedStyle&) const { return 0; }
virtual int popupInternalPaddingEnd(const HostWindow*,
const ComputedStyle&) const {
return 0;
}
virtual int popupInternalPaddingTop(const ComputedStyle&) const { return 0; }
virtual int popupInternalPaddingBottom(const ComputedStyle&) const {
return 0;
......
third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp
View file @ 0151427d
......@@ -28,6 +28,7 @@
#include "core/layout/LayoutThemeFontProvider.h"
#include "core/paint/MediaControlsPainter.h"
#include "core/style/ComputedStyle.h"
#include "platform/HostWindow.h"
#include "platform/LayoutTestSupport.h"
#include "platform/PlatformResourceLoader.h"
#include "platform/graphics/Color.h"
......@@ -42,7 +43,6 @@ static const float defaultControlFontPixelSize = 13;
static const float defaultCancelButtonSize = 9;
static const float minCancelButtonSize = 5;
static const float maxCancelButtonSize = 21;
static const int menuListArrowPaddingSize = 14;
static bool useMockTheme() {
return LayoutTestSupport::isMockThemeEnabledForTest();
......@@ -55,7 +55,8 @@ unsigned LayoutThemeDefault::m_inactiveSelectionForegroundColor = 0xff323232;
double LayoutThemeDefault::m_caretBlinkInterval;
LayoutThemeDefault::LayoutThemeDefault() : LayoutTheme(nullptr) {
LayoutThemeDefault::LayoutThemeDefault()
: LayoutTheme(nullptr), m_painter(*this) {
m_caretBlinkInterval = LayoutTheme::caretBlinkInterval();
}
......@@ -316,8 +317,12 @@ int LayoutThemeDefault::popupInternalPaddingStart(
}
int LayoutThemeDefault::popupInternalPaddingEnd(
const HostWindow* host,
const ComputedStyle& style) const {
return menuListInternalPadding(style, 4 + menuListArrowPaddingSize);
if (style.appearance() == NoControlPart)
return 0;
return 1 * style.effectiveZoom() +
clampedMenuListArrowPaddingSize(host, style);
}
int LayoutThemeDefault::popupInternalPaddingTop(
......@@ -330,6 +335,32 @@ int LayoutThemeDefault::popupInternalPaddingBottom(
return menuListInternalPadding(style, 1);
}
// static
int LayoutThemeDefault::scrollbarThicknessInDIP() {
int width = Platform::current()
->themeEngine()
->getSize(WebThemeEngine::PartScrollbarDownArrow)
.width;
return width > 0 ? width : 15;
}
// static
float LayoutThemeDefault::clampedMenuListArrowPaddingSize(
const HostWindow* host,
const ComputedStyle& style) {
int originalSize = scrollbarThicknessInDIP();
int scaledSize =
host ? host->windowToViewportScalar(originalSize) : originalSize;
// The result should not be samller than the scrollbar thickness in order to
// secure space for scrollbar in popup.
float deviceScale = 1.0f * scaledSize / originalSize;
if (style.effectiveZoom() < deviceScale)
return scaledSize;
// The value should be zoomed though scrollbars aren't scaled by zoom.
// crbug.com/432795.
return originalSize * style.effectiveZoom();
}
// static
void LayoutThemeDefault::setDefaultFontSize(int fontSize) {
LayoutThemeFontProvider::setDefaultFontSize(fontSize);
......
third_party/WebKit/Source/core/layout/LayoutThemeDefault.h
View file @ 0151427d
......@@ -111,9 +111,13 @@ class CORE_EXPORT LayoutThemeDefault : public LayoutTheme {
// These methods define the padding for the MenuList's inner block.
int popupInternalPaddingStart(const ComputedStyle&) const override;
int popupInternalPaddingEnd(const ComputedStyle&) const override;
int popupInternalPaddingEnd(const HostWindow*,
const ComputedStyle&) const override;
int popupInternalPaddingTop(const ComputedStyle&) const override;
int popupInternalPaddingBottom(const ComputedStyle&) const override;
static int scrollbarThicknessInDIP();
static float clampedMenuListArrowPaddingSize(const HostWindow*,
const ComputedStyle&);
// Provide a way to pass the default font size from the Settings object
// to the layout theme. FIXME: http://b/1129186 A cleaner way would be
......
third_party/WebKit/Source/core/layout/LayoutThemeMac.h
View file @ 0151427d
......@@ -74,7 +74,8 @@ class LayoutThemeMac final : public LayoutTheme {
int sliderTickOffsetFromTrackCenter() const override;
int popupInternalPaddingStart(const ComputedStyle&) const override;
int popupInternalPaddingEnd(const ComputedStyle&) const override;
int popupInternalPaddingEnd(const HostWindow*,
const ComputedStyle&) const override;
int popupInternalPaddingTop(const ComputedStyle&) const override;
int popupInternalPaddingBottom(const ComputedStyle&) const override;
......
third_party/WebKit/Source/core/layout/LayoutThemeMac.mm
View file @ 0151427d
......@@ -754,7 +754,8 @@ int LayoutThemeMac::popupInternalPaddingStart(
return 0;
}
int LayoutThemeMac::popupInternalPaddingEnd(const ComputedStyle& style) const {
int LayoutThemeMac::popupInternalPaddingEnd(const HostWindow*,
const ComputedStyle& style) const {
if (style.appearance() == MenulistPart)
return popupButtonPadding(
controlSizeForFont(style))[ThemeMac::RightMargin] *
......
third_party/WebKit/Source/core/paint/ThemePainterDefault.cpp
View file @ 0151427d
......@@ -24,9 +24,10 @@
#include "core/paint/ThemePainterDefault.h"
#include "core/frame/FrameView.h"
#include "core/layout/LayoutObject.h"
#include "core/layout/LayoutProgress.h"
#include "core/layout/LayoutTheme.h"
#include "core/layout/LayoutThemeDefault.h"
#include "core/paint/MediaControlsPainter.h"
#include "core/paint/PaintInfo.h"
#include "platform/LayoutTestSupport.h"
......@@ -42,10 +43,6 @@ namespace blink {
namespace {
const unsigned defaultButtonBackgroundColor = 0xffdddddd;
const unsigned mockDropdownMenuListArrowPadding = 3;
// This is equal to the padding provided by the LayoutMenuList which is
// calculated based on |menuListArrowPaddingSize| in LayoutThemeDefault.
const unsigned mockDropdownMenuListArrowWidth = 18;
bool useMockTheme() {
return LayoutTestSupport::isMockThemeEnabledForTest();
......@@ -142,7 +139,8 @@ IntRect convertToPaintingRect(const LayoutObject& inputLayoutObject,
} // namespace
ThemePainterDefault::ThemePainterDefault() : ThemePainter() {}
ThemePainterDefault::ThemePainterDefault(LayoutThemeDefault& theme)
: ThemePainter(), m_theme(theme) {}
bool ThemePainterDefault::paintCheckbox(const LayoutObject& o,
const PaintInfo& i,
......@@ -285,34 +283,40 @@ void ThemePainterDefault::setupMenuListArrow(
const LayoutBox& box,
const IntRect& rect,
WebThemeEngine::ExtraParams& extraParams) {
const int right = rect.x() + rect.width();
const int left = rect.x() + box.borderLeft();
const int right = rect.x() + rect.width() - box.borderRight();
const int middle = rect.y() + rect.height() / 2;
extraParams.menuList.arrowY = middle;
float arrowBoxWidth = m_theme.clampedMenuListArrowPaddingSize(
box.frameView()->getHostWindow(), box.styleRef());
float arrowScaleFactor = arrowBoxWidth / m_theme.scrollbarThicknessInDIP();
if (useMockTheme()) {
// The size and position of the drop-down button is different between
// the mock theme and the regular aura theme.
int extraPadding =
mockDropdownMenuListArrowPadding * box.styleRef().effectiveZoom();
int arrowBoxWidth =
mockDropdownMenuListArrowWidth * box.styleRef().effectiveZoom();
int arrowSize = std::min(arrowBoxWidth, rect.height()) - 2 * extraPadding;
// Padding inside the arrowBox.
float extraPadding = 2 * arrowScaleFactor;
float arrowSize =
std::min(arrowBoxWidth,
static_cast<float>(rect.height() - box.borderTop() -
box.borderBottom())) -
2 * extraPadding;
// |arrowX| is the middle position for mock theme engine.
extraParams.menuList.arrowX =
(box.styleRef().direction() == RTL)
? rect.x() + extraPadding + (arrowSize / 2)
: right - (arrowSize / 2) - extraPadding;
extraParams.menuList.arrowSize = arrowSize;
} else {
const int arrowSize = 6;
const int arrowPadding = 6;
extraParams.menuList.arrowX =
(box.styleRef().direction() == RTL)
? rect.x() + arrowPadding * box.styleRef().effectiveZoom() +
box.borderLeft()
: right -
(arrowSize + arrowPadding) * box.styleRef().effectiveZoom() -
box.borderRight();
extraParams.menuList.arrowSize = arrowSize * box.styleRef().effectiveZoom();
// TODO(tkent): This should be 7.0 to match scroll bar buttons.
float arrowSize = 6.0 * arrowScaleFactor;
// Put the 6px arrow at the center of paddingForArrow area.
// |arrowX| is the left position for Aura theme engine.
extraParams.menuList.arrowX = (box.styleRef().direction() == RTL)
? left + (arrowBoxWidth - arrowSize) / 2
: right - (arrowBoxWidth + arrowSize) / 2;
extraParams.menuList.arrowSize = arrowSize;
}
extraParams.menuList.arrowColor = box.resolveColor(CSSPropertyColor).rgb();
}
......
third_party/WebKit/Source/core/paint/ThemePainterDefault.h
View file @ 0151427d
......@@ -35,10 +35,11 @@
namespace blink {
class LayoutBox;
class LayoutThemeDefault;
class ThemePainterDefault final : public ThemePainter {
public:
ThemePainterDefault();
explicit ThemePainterDefault(LayoutThemeDefault&);
private:
bool paintCheckbox(const LayoutObject&,
......@@ -84,6 +85,9 @@ class ThemePainterDefault final : public ThemePainter {
void setupMenuListArrow(const LayoutBox&,
const IntRect&,
WebThemeEngine::ExtraParams&);
// ThemePaintDefault is a part object of m_theme.
LayoutThemeDefault& m_theme;
};
} // namespace blink
......
third_party/WebKit/Source/platform/scroll/Scrollbar.h
View file @ 0151427d
......@@ -108,6 +108,9 @@ class PLATFORM_EXPORT Scrollbar : public Widget,
bool enabled() const override { return m_enabled; }
void setEnabled(bool) override;
// This returns device-scale-factor-aware pixel value.
// e.g. 15 in dsf=1.0, 30 in dsf=2.0.
// See also ScrolbarTheme::scrollbatThickness().
int scrollbarThickness() const;
// Called by the ScrollableArea when the scroll offset changes.
......
third_party/WebKit/Source/platform/scroll/ScrollbarTheme.h
View file @ 0151427d
......@@ -58,6 +58,8 @@ class PLATFORM_EXPORT ScrollbarTheme {
virtual ScrollbarPart hitTest(const ScrollbarThemeClient&, const IntPoint&);
// This returns a fixed value regardless of device-scale-factor.
// See also Scrollbar::scrollbarThickness().
virtual int scrollbarThickness(ScrollbarControlSize = RegularScrollbar) {
return 0;
}
......
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