Skip to content

Commit

Permalink
REGRESSION (Safari 16.4): Dynamically set <meter> doesn't show the …
Browse files Browse the repository at this point in the history
…value

https://bugs.webkit.org/show_bug.cgi?id=255914
rdar://108495001

Reviewed by Tim Horton.

257981@main began the process of refactoring form control rendering to support
the GPU process on macOS. Since the refactoring, form controls are represented
by various `ControlPart` subclasses, which can be drawn using a `GraphicsContext`.

For the more customizable controls (such as <meter>, <progress>,
<input type=range>, and Apple Pay buttons), form control state is encoded directly
on the `ControlPart` subclass. The `ControlPart` is only created once per renderer
in the various flavors of `RenderTheme::ensureControlPart`. Consequently, the
a form control's updated state is never reflected in the `ControlPart` once it
has been created. This results in dynamic changes to state not being reflected
in painting.

To fix, ensure the `ControlPart` is up-to-date with the latest state prior to
painting.

* LayoutTests/TestExpectations:
* LayoutTests/fast/css/appearance-apple-pay-button-style-dynamic-update-expected.html: Added.
* LayoutTests/fast/css/appearance-apple-pay-button-style-dynamic-update.html: Added.
* LayoutTests/fast/forms/meter-dynamic-update-expected.html: Added.
* LayoutTests/fast/forms/meter-dynamic-update.html: Added.
* LayoutTests/fast/forms/progress-dynamic-update-expected.html: Added.
* LayoutTests/fast/forms/progress-dynamic-update.html: Added.
* LayoutTests/fast/forms/range-datalist-dynamic-update-expected.html: Added.
* LayoutTests/fast/forms/range-datalist-dynamic-update.html: Added.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:
* Source/WebCore/platform/graphics/controls/ApplePayButtonPart.cpp:
(WebCore::ApplePayButtonPart::create):
* Source/WebCore/platform/graphics/controls/ApplePayButtonPart.h:
(WebCore::ApplePayButtonPart::setButtonType):
(WebCore::ApplePayButtonPart::setButtonStyle):
(WebCore::ApplePayButtonPart::setLocale):
* Source/WebCore/platform/graphics/controls/MeterPart.cpp:
(WebCore::MeterPart::create):
* Source/WebCore/platform/graphics/controls/MeterPart.h:
(WebCore::MeterPart::setGaugeRegion):
(WebCore::MeterPart::setValue):
(WebCore::MeterPart::setMinimum):
(WebCore::MeterPart::setMaximum):
* Source/WebCore/platform/graphics/controls/ProgressBarPart.cpp:
(WebCore::ProgressBarPart::create):
* Source/WebCore/platform/graphics/controls/ProgressBarPart.h:
(WebCore::ProgressBarPart::setPosition):
(WebCore::ProgressBarPart::setAnimationStartTime):
* Source/WebCore/platform/graphics/controls/SliderTrackPart.cpp:
(WebCore::SliderTrackPart::create):
* Source/WebCore/platform/graphics/controls/SliderTrackPart.h:
(WebCore::SliderTrackPart::setThumbSize):
(WebCore::SliderTrackPart::setTrackBounds):
(WebCore::SliderTrackPart::setTickRatios):
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::updateApplePayButtonPartForRenderer):
(WebCore::updateMeterPartForRenderer):
(WebCore::updateProgressBarPartForRenderer):
(WebCore::updateSliderTrackPartForRenderer):
(WebCore::RenderTheme::createControlPart const):

Don't capture any state on creation, it is only necessary for painting, and
will be set prior to drawing.

(WebCore::RenderTheme::updateControlPartForRenderer const):

Update the `ControlPart` based on the latest state, only for controls that
require additional state.

(WebCore::RenderTheme::paint):

Ensure the `ControlPart` state is up-to-date prior to painting.

(WebCore::createApplePayButtonPartForRenderer): Deleted.
(WebCore::createMeterPartForRenderer): Deleted.
(WebCore::createProgressBarPartForRenderer): Deleted.
(WebCore::createSliderTrackPartForRenderer): Deleted.
* Source/WebCore/rendering/RenderTheme.h:

Canonical link: https://commits.webkit.org/263473@main
  • Loading branch information
pxlcoder committed Apr 27, 2023
1 parent 3fca1b3 commit 340e63b
Show file tree
Hide file tree
Showing 21 changed files with 208 additions and 20 deletions.
1 change: 1 addition & 0 deletions LayoutTests/TestExpectations
Expand Up @@ -285,6 +285,7 @@ fast/css/appearance-apple-pay-button-border-radius.html [ Skip ]
fast/css/appearance-apple-pay-button-border-radius-longhands.html [ Skip ]
fast/css/appearance-apple-pay-button-border-width.html [ Skip ]
fast/css/appearance-apple-pay-button-default-corners.html [ Skip ]
fast/css/appearance-apple-pay-button-style-dynamic-update.html [ Skip ]
fast/css/getComputedStyle/computed-style-apple-pay-button.html [ Skip ]
fast/css/webkit-named-image/apple-pay-logo-black [ Skip ]
fast/css/webkit-named-image/apple-pay-logo-white [ Skip ]
Expand Down
@@ -0,0 +1 @@
<button id="button" style="-webkit-appearance: -apple-pay-button; -apple-pay-button-style: white;"></button>
@@ -0,0 +1,33 @@
<style>

#button {
-webkit-appearance: -apple-pay-button;
}

.white {
-apple-pay-button-style: white;
}

.black {
-apple-pay-button-style: black;
}

</style>
<script>

if (window.testRunner)
testRunner.waitUntilDone();

function load()
{
setTimeout(() => {
button.className = "white";

if (window.testRunner)
testRunner.notifyDone();
}, 0);
}

</script>
<body onload="load()">
<button id="button" class="black"></button>
1 change: 1 addition & 0 deletions LayoutTests/fast/forms/meter-dynamic-update-expected.html
@@ -0,0 +1 @@
<meter id="meter" max="100" low="20" high="80" optimum="90" value="18"></meter>
22 changes: 22 additions & 0 deletions LayoutTests/fast/forms/meter-dynamic-update.html
@@ -0,0 +1,22 @@
<script>

if (window.testRunner)
testRunner.waitUntilDone();

function load()
{
setTimeout(() => {
meter.max = 100;
meter.low = 20;
meter.high = 80;
meter.optimum = 90;
meter.value = 18;

if (window.testRunner)
testRunner.notifyDone();
}, 0);
}

</script>
<body onload="load()">
<meter id="meter"></meter>
@@ -0,0 +1 @@
<progress id="progress" max="100" value="70"></progress>
18 changes: 18 additions & 0 deletions LayoutTests/fast/forms/progress-dynamic-update.html
@@ -0,0 +1,18 @@
<script>

if (window.testRunner)
testRunner.waitUntilDone();

function load()
{
setTimeout(() => {
progress.value = 70;

if (window.testRunner)
testRunner.notifyDone();
}, 0);
}

</script>
<body onload="load()">
<progress id="progress" max="100" value="50"></progress>
@@ -0,0 +1,8 @@
<input type="range" id="range" list="markers">
<datalist id="markers">
<option value="0"></option>
<option value="25"></option>
<option value="50"></option>
<option value="75"></option>
<option value="100"></option>
</datalist>
25 changes: 25 additions & 0 deletions LayoutTests/fast/forms/range-datalist-dynamic-update.html
@@ -0,0 +1,25 @@
<script>

if (window.testRunner)
testRunner.waitUntilDone();

function load()
{
setTimeout(() => {
range.setAttribute("list", "markers");

if (window.testRunner)
testRunner.notifyDone();
}, 0);
}

</script>
<body onload="load()">
<input type="range" id="range">
<datalist id="markers">
<option value="0"></option>
<option value="25"></option>
<option value="50"></option>
<option value="75"></option>
<option value="100"></option>
</datalist>
1 change: 1 addition & 0 deletions LayoutTests/platform/ios/TestExpectations
Expand Up @@ -41,6 +41,7 @@ fast/css/appearance-apple-pay-button-border-radius.html [ Pass ]
fast/css/appearance-apple-pay-button-border-radius-longhands.html [ Pass ]
fast/css/appearance-apple-pay-button-border-width.html [ Pass ]
fast/css/appearance-apple-pay-button-default-corners.html [ Pass ]
fast/css/appearance-apple-pay-button-style-dynamic-update.html [ Pass ]
fast/css/webkit-named-image/apple-pay-logo-black [ Pass ]
fast/css/webkit-named-image/apple-pay-logo-white [ Pass ]
fast/css/getComputedStyle/computed-style-apple-pay-button.html [ Pass ]
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/platform/mac/TestExpectations
Expand Up @@ -1072,6 +1072,7 @@ fast/css/appearance-apple-pay-button-border-radius.html [ Pass ]
fast/css/appearance-apple-pay-button-border-radius-longhands.html [ Pass ]
fast/css/appearance-apple-pay-button-border-width.html [ Pass ]
fast/css/appearance-apple-pay-button-default-corners.html [ Pass ]
fast/css/appearance-apple-pay-button-style-dynamic-update.html [ Pass ]
fast/css/webkit-named-image/apple-pay-logo-black [ Pass ]
fast/css/webkit-named-image/apple-pay-logo-white [ Pass ]
fast/css/getComputedStyle/computed-style-apple-pay-button.html [ Pass ]
Expand Down
Expand Up @@ -32,6 +32,11 @@

namespace WebCore {

Ref<ApplePayButtonPart> ApplePayButtonPart::create()
{
return adoptRef(*new ApplePayButtonPart(ApplePayButtonType::Plain, ApplePayButtonStyle::White, { }));
}

Ref<ApplePayButtonPart> ApplePayButtonPart::create(ApplePayButtonType buttonType, ApplePayButtonStyle buttonStyle, const String& locale)
{
return adoptRef(*new ApplePayButtonPart(buttonType, buttonStyle, locale));
Expand Down
Expand Up @@ -59,11 +59,17 @@ enum class ApplePayButtonStyle : uint8_t {

class ApplePayButtonPart : public ControlPart {
public:
static Ref<ApplePayButtonPart> create();
WEBCORE_EXPORT static Ref<ApplePayButtonPart> create(ApplePayButtonType, ApplePayButtonStyle, const String& locale);

ApplePayButtonType buttonType() const { return m_buttonType; }
void setButtonType(ApplePayButtonType buttonType) { m_buttonType = buttonType; }

ApplePayButtonStyle buttonStyle() const { return m_buttonStyle; }
void setButtonStyle(ApplePayButtonStyle buttonStyle) { m_buttonStyle = buttonStyle; }

String locale() const { return m_locale; }
void setLocale(String locale) { m_locale = locale; }

private:
ApplePayButtonPart(ApplePayButtonType, ApplePayButtonStyle, const String& locale);
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/platform/graphics/controls/MeterPart.cpp
Expand Up @@ -30,6 +30,11 @@

namespace WebCore {

Ref<MeterPart> MeterPart::create()
{
return adoptRef(*new MeterPart(GaugeRegion::EvenLessGood, 0, 0, 0));
}

Ref<MeterPart> MeterPart::create(GaugeRegion gaugeRegion, double value, double minimum, double maximum)
{
return adoptRef(*new MeterPart(gaugeRegion, value, minimum, maximum));
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/platform/graphics/controls/MeterPart.h
Expand Up @@ -37,12 +37,20 @@ class MeterPart : public ControlPart {
EvenLessGood
};

static Ref<MeterPart> create();
WEBCORE_EXPORT static Ref<MeterPart> create(GaugeRegion, double value, double minimum, double maximum);

GaugeRegion gaugeRegion() const { return m_gaugeRegion; }
void setGaugeRegion(GaugeRegion gaugeRegion) { m_gaugeRegion = gaugeRegion; }

double value() const { return m_value; }
void setValue(double value) { m_value = value; }

double minimum() const { return m_minimum; }
void setMinimum(double minimum) { m_minimum = minimum; }

double maximum() const { return m_maximum; }
void setMaximum(double maximum) { m_maximum = maximum; }

private:
MeterPart(GaugeRegion, double value, double minimum, double maximum);
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/platform/graphics/controls/ProgressBarPart.cpp
Expand Up @@ -30,6 +30,11 @@

namespace WebCore {

Ref<ProgressBarPart> ProgressBarPart::create()
{
return adoptRef(*new ProgressBarPart(0, 0_s));
}

Ref<ProgressBarPart> ProgressBarPart::create(double position, const Seconds& animationStartTime)
{
return adoptRef(*new ProgressBarPart(position, animationStartTime));
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/platform/graphics/controls/ProgressBarPart.h
Expand Up @@ -32,10 +32,14 @@ namespace WebCore {

class ProgressBarPart : public ControlPart {
public:
static Ref<ProgressBarPart> create();
WEBCORE_EXPORT static Ref<ProgressBarPart> create(double position, const Seconds& animationStartTime);

double position() const { return m_position; }
void setPosition(double position) { m_position = position; }

Seconds animationStartTime() const { return m_animationStartTime; }
void setAnimationStartTime(Seconds animationStartTime) { m_animationStartTime = animationStartTime; }

private:
ProgressBarPart(double position, const Seconds& animationStartTime);
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/platform/graphics/controls/SliderTrackPart.cpp
Expand Up @@ -31,6 +31,11 @@

namespace WebCore {

Ref<SliderTrackPart> SliderTrackPart::create(StyleAppearance type)
{
return adoptRef(*new SliderTrackPart(type, { }, { }, { }));
}

Ref<SliderTrackPart> SliderTrackPart::create(StyleAppearance type, const IntSize& thumbSize, const IntRect& trackBounds, Vector<double>&& tickRatios)
{
return adoptRef(*new SliderTrackPart(type, thumbSize, trackBounds, WTFMove(tickRatios)));
Expand Down
6 changes: 6 additions & 0 deletions Source/WebCore/platform/graphics/controls/SliderTrackPart.h
Expand Up @@ -32,11 +32,17 @@ namespace WebCore {

class SliderTrackPart : public ControlPart {
public:
static Ref<SliderTrackPart> create(StyleAppearance);
WEBCORE_EXPORT static Ref<SliderTrackPart> create(StyleAppearance, const IntSize& thumbSize, const IntRect& trackBounds, Vector<double>&& tickRatios);

IntSize thumbSize() const { return m_thumbSize; }
void setThumbSize(IntSize thumbSize) { m_thumbSize = thumbSize; }

IntRect trackBounds() const { return m_trackBounds; }
void setTrackBounds(IntRect trackBounds) { m_trackBounds = trackBounds; }

const Vector<double>& tickRatios() const { return m_tickRatios; }
void setTickRatios(Vector<double>&& tickRatios) { m_tickRatios = WTFMove(tickRatios); }

#if ENABLE(DATALIST_ELEMENT)
void drawTicks(GraphicsContext&, const FloatRect&, const ControlStyle&) const;
Expand Down

0 comments on commit 340e63b

Please sign in to comment.