New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 699884 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocking:
issue 595137
issue 695267



Sign in to add a comment

Missing sibling invalidation for :in-range on value attribute changes

Project Member Reported by nainar@chromium.org, Mar 9 2017

Issue description

Please see test case code here:
<!DOCTYPE html>
<style>
    .inrange {
        background-color: red
    }
    input:in-range + .inrange {
        background-color: green
    }
</style>
<div>
    <input type="number" min="1" max="10" value="20" id="t5">
    <span id="r5" class="inrange">This text should have a green background</span>
</div>
<script>
    getComputedStyle(r5).backgroundColor;
    t5.setAttribute("value", "5");
</script>


While in FF we do recalc style once we change the attribute we don't do so in Chrome. I suspect SiblingInvalidationSets are broken. 

This is blocking Squad Stage 2a. 
 
Summary: Missing sibling invalidation for :in-range on value attribute changes (was: Not computing style again if we chane attribute of an element)

Comment 2 by r...@opera.com, Mar 9 2017

Cc: tkent@chromium.org
Components: -Blink>CSS Blink>Forms
pseudoStateChanged() is not called from HTMLFormControlElement::setNeedsValidityCheck() because m_validityIsDirty is already true when setAttribute() is called on t5. That also happens if I delay the setAttribute with a fairly long timeout.

Comment 3 by r...@opera.com, Mar 9 2017

Cc: -tkent@chromium.org r...@opera.com
Owner: tkent@chromium.org
HTMLFormControlElement::setNeedsWillValidateCheck() has some comment about forcing m_validityIsDirty = false for FORM/FIELDSET in order to trigger style changes. I believe this is a similar issue. I don't know how this is supposed to work. tkent@?

Comment 4 by tkent@chromium.org, Mar 13 2017

The root problem is that m_validityIsDirty controls isValidElement(), which is the source of :valid and :invalid, and it's not related to isInRange() and isOutOfRange() for :in-range and :out-of-range.
We just need to move pseudoStateChanged() for InRange and OutOfRange out from the |if (!m_validityisDirty)| block in setNeedsValidityCheck().

Comment 5 by r...@opera.com, Mar 21 2017

Cc: -r...@opera.com tkent@chromium.org
Owner: r...@opera.com
Status: Started (was: Assigned)
https://codereview.chromium.org/2764023003/
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df2ef06a75aae87d449e2f801e90fd418679871b

commit df2ef06a75aae87d449e2f801e90fd418679871b
Author: rune <rune@opera.com>
Date: Tue Mar 21 22:13:56 2017

Updating :in-range should not rely on validation.

m_validityIsDirty is not related to isInRange() and isOutOfRange().
This fixes sibling style invalidation using invalidation sets for
:in-range and :out-of-range changes when value changes through
setAttribute.

R=tkent@chromium.org
BUG= 699884 

Review-Url: https://codereview.chromium.org/2764023003
Cr-Commit-Position: refs/heads/master@{#458576}

[add] https://crrev.com/df2ef06a75aae87d449e2f801e90fd418679871b/third_party/WebKit/LayoutTests/fast/css/invalidation/in-range-pseudo.html
[modify] https://crrev.com/df2ef06a75aae87d449e2f801e90fd418679871b/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp

Comment 7 by r...@opera.com, Mar 21 2017

Status: Fixed (was: Started)

Comment 8 by tkent@chromium.org, Mar 24 2017

Labels: -Pri-3 Merge-Request-58 Pri-2

Comment 9 by tkent@chromium.org, Mar 24 2017

Labels: -Type-Bug Hotlist-Interop Type-Bug-Regression
Project Member

Comment 10 by sheriffbot@chromium.org, Mar 24 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 24 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/660abd2962f7bf553dd7f4146f1ec5e5f1f168d0

commit 660abd2962f7bf553dd7f4146f1ec5e5f1f168d0
Author: Kent Tamura <tkent@chromium.org>
Date: Fri Mar 24 08:17:55 2017

Merge "Updating :in-range should not rely on validation." to M58.

m_validityIsDirty is not related to isInRange() and isOutOfRange().
This fixes sibling style invalidation using invalidation sets for
:in-range and :out-of-range changes when value changes through
setAttribute.

R=tkent@chromium.org
BUG= 699884 

Review-Url: https://codereview.chromium.org/2764023003
Cr-Commit-Position: refs/heads/master@{#458576}
(cherry picked from commit df2ef06a75aae87d449e2f801e90fd418679871b)

Review-Url: https://codereview.chromium.org/2777543002 .
Cr-Commit-Position: refs/branch-heads/3029@{#404}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[add] https://crrev.com/660abd2962f7bf553dd7f4146f1ec5e5f1f168d0/third_party/WebKit/LayoutTests/fast/css/invalidation/in-range-pseudo.html
[modify] https://crrev.com/660abd2962f7bf553dd7f4146f1ec5e5f1f168d0/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp

Comment 12 by tkent@chromium.org, Mar 24 2017

Labels: M-58
Labels: TE-Verified-M58 TE-Verified-58.0.3029.41
Verified the issue on windows 10, Mac 10.12.3 and Ubuntu 14.04 using chrome beta version #58.0.3029.41 as per comment #0

Observed that the text has a green background as expected. Hence, the fix is working as expected.

Attaching screen shot for reference.

Hence, adding the verified labels.

Thanks...!!
699884.JPG
82.3 KB View Download

Sign in to add a comment