⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 267721 in webkit


Ignore:
Timestamp:
Sep 28, 2020, 5:38:15 PM (6 years ago)
Author:
Alan Coon
Message:

Cherry-pick r266887. rdar://problem/69586659

Integrator's note: as some of the symbols present on trunk are not available on branch, special modifications had to be made to this cherry-pick to build.

REGRESSION (r257839): clickpay.com - password placeholder text cannot be replaced
​https://bugs.webkit.org/show_bug.cgi?id=216257
<rdar://problem/68150686>

Reviewed by Antti Koivisto.

Source/WebCore:

On clickpay.com, the field in the login form that contains the text "Password" is actually a plain text input,
referred to by the site's script as the "null text input". The page adds a focus event listener to this null
text input, and inside of this focus event listener, it reveals a hidden password field by removing an inline
display: none; style rule on the real password input element, programmatically focuses it, and then hides the
null text input by setting it to display: none; via inline style.

However, after the changes in r257839, we no longer attempt to do a style update upon programmatic focus in the
case where the programmatically focused element does not have a renderer yet (this applies to the password field
in this scenario, because it previously had display: none;). When we determine whether the newly displayed
password field is focusable using Element::isVisibleWithoutResolvingFullStyle, we then attempt to use either
the existing computed RenderStyle on the element, or perform a partial computed style resolution using the
ResolveComputedStyleMode::RenderedOnly flag.

But in the case where ElementRareData's computed style exists, it is not guaranteed to be up to date if the
inline style changed since the computed style was last set. In the context of this bug, it's actually Safari's
AutoFill logic (embedded in the injected bundle) that ends up asking for the computed style of the password
input, forcing it to be created and set (though, as demonstrated in the layout test, simply grabbing the
computed style is sufficient to replicate the bug outside of Safari).

The end result is that we'll use this stale computed style, which still believes that the password input is not
displayed, and we end up not focusing the element due to believing that the password input is hidden. To fix
this, we would need to either check whether the element has an invalid style (i.e. needsStyleRecalc()) before
attempting to use the existing computed style, or clear out the ElementRareData computed style anytime the
element's style is invalidated. However, both of these approaches will cause us to perform partial style
resolution much more aggressively, leading to a 2-3% regression in Speedometer.

To address the bug without hampering our performance wins from r257839, we add a new node flag so that we can
remember when computed styles are no longer valid due to style invalidation, and consult this flag in
Element::isVisibleWithoutResolvingFullStyle to avoid using the existing computed style.

Test: fast/forms/programmatic-focus-after-display.html

  • dom/Element.cpp: (WebCore::Element::invalidateStyle): (WebCore::Element::resolveComputedStyle): (WebCore::Element::isVisibleWithoutResolvingFullStyle const):
  • dom/Node.h: (WebCore::Node::setHasValidStyle):

LayoutTests:

Add a new layout test to verify that the bug does not occur. See WebCore/ChangeLog for more details.

  • fast/forms/programmatic-focus-after-display-expected.txt: Added.
  • fast/forms/programmatic-focus-after-display.html: Added.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@266887 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-610-branch
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-610-branch/LayoutTests/ChangeLog

    r267720 r267721  
     12020-09-28  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r266887. rdar://problem/69586659
     4
     5    REGRESSION (r257839): clickpay.com - password placeholder text cannot be replaced
     6    https://bugs.webkit.org/show_bug.cgi?id=216257
     7    <rdar://problem/68150686>
     8   
     9    Reviewed by Antti Koivisto.
     10   
     11    Source/WebCore:
     12   
     13    On clickpay.com, the field in the login form that contains the text "Password" is actually a plain text input,
     14    referred to by the site's script as the "null text input". The page adds a focus event listener to this null
     15    text input, and inside of this focus event listener, it reveals a hidden password field by removing an inline
     16    `display: none;` style rule on the real password input element, programmatically focuses it, and then hides the
     17    null text input by setting it to `display: none;` via inline style.
     18   
     19    However, after the changes in r257839, we no longer attempt to do a style update upon programmatic focus in the
     20    case where the programmatically focused element does not have a renderer yet (this applies to the password field
     21    in this scenario, because it previously had `display: none;`). When we determine whether the newly displayed
     22    password field is focusable using `Element::isVisibleWithoutResolvingFullStyle`, we then attempt to use either
     23    the existing computed `RenderStyle` on the element, or perform a partial computed style resolution using the
     24    `ResolveComputedStyleMode::RenderedOnly` flag.
     25   
     26    But in the case where `ElementRareData`'s computed style exists, it is not guaranteed to be up to date if the
     27    inline style changed since the computed style was last set. In the context of this bug, it's actually Safari's
     28    AutoFill logic (embedded in the injected bundle) that ends up asking for the computed style of the password
     29    input, forcing it to be created and set (though, as demonstrated in the layout test, simply grabbing the
     30    computed style is sufficient to replicate the bug outside of Safari).
     31   
     32    The end result is that we'll use this stale computed style, which still believes that the password input is not
     33    displayed, and we end up not focusing the element due to believing that the password input is hidden. To fix
     34    this, we would need to either check whether the element has an invalid style (i.e. `needsStyleRecalc()`) before
     35    attempting to use the existing computed style, or clear out the `ElementRareData` computed style anytime the
     36    element's style is invalidated. However, both of these approaches will cause us to perform partial style
     37    resolution much more aggressively, leading to a 2-3% regression in Speedometer.
     38   
     39    To address the bug without hampering our performance wins from r257839, we add a new node flag so that we can
     40    remember when computed styles are no longer valid due to style invalidation, and consult this flag in
     41    `Element::isVisibleWithoutResolvingFullStyle` to avoid using the existing computed style.
     42   
     43    Test: fast/forms/programmatic-focus-after-display.html
     44   
     45    * dom/Element.cpp:
     46    (WebCore::Element::invalidateStyle):
     47    (WebCore::Element::resolveComputedStyle):
     48    (WebCore::Element::isVisibleWithoutResolvingFullStyle const):
     49    * dom/Node.h:
     50    (WebCore::Node::setHasValidStyle):
     51   
     52    LayoutTests:
     53   
     54    Add a new layout test to verify that the bug does not occur. See WebCore/ChangeLog for more details.
     55   
     56    * fast/forms/programmatic-focus-after-display-expected.txt: Added.
     57    * fast/forms/programmatic-focus-after-display.html: Added.
     58   
     59    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266887 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     60
     61    2020-09-10  Wenson Hsieh  <wenson_hsieh@apple.com>
     62
     63            REGRESSION (r257839): clickpay.com - password placeholder text cannot be replaced
     64            https://bugs.webkit.org/show_bug.cgi?id=216257
     65            <rdar://problem/68150686>
     66
     67            Reviewed by Antti Koivisto.
     68
     69            Add a new layout test to verify that the bug does not occur. See WebCore/ChangeLog for more details.
     70
     71            * fast/forms/programmatic-focus-after-display-expected.txt: Added.
     72            * fast/forms/programmatic-focus-after-display.html: Added.
     73
    1742020-09-28  Karl Rackler  <rackler@apple.com>
    275

  • branches/safari-610-branch/Source/WebCore/ChangeLog

    r267716 r267721  
     12020-09-28  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r266887. rdar://problem/69586659
     4
     5    Integrator's note: as some of the symbols present on trunk are not available on branch, special modifications had to be made to this cherry-pick to build.
     6
     7    REGRESSION (r257839): clickpay.com - password placeholder text cannot be replaced
     8    https://bugs.webkit.org/show_bug.cgi?id=216257
     9    <rdar://problem/68150686>
     10   
     11    Reviewed by Antti Koivisto.
     12   
     13    Source/WebCore:
     14   
     15    On clickpay.com, the field in the login form that contains the text "Password" is actually a plain text input,
     16    referred to by the site's script as the "null text input". The page adds a focus event listener to this null
     17    text input, and inside of this focus event listener, it reveals a hidden password field by removing an inline
     18    `display: none;` style rule on the real password input element, programmatically focuses it, and then hides the
     19    null text input by setting it to `display: none;` via inline style.
     20   
     21    However, after the changes in r257839, we no longer attempt to do a style update upon programmatic focus in the
     22    case where the programmatically focused element does not have a renderer yet (this applies to the password field
     23    in this scenario, because it previously had `display: none;`). When we determine whether the newly displayed
     24    password field is focusable using `Element::isVisibleWithoutResolvingFullStyle`, we then attempt to use either
     25    the existing computed `RenderStyle` on the element, or perform a partial computed style resolution using the
     26    `ResolveComputedStyleMode::RenderedOnly` flag.
     27   
     28    But in the case where `ElementRareData`'s computed style exists, it is not guaranteed to be up to date if the
     29    inline style changed since the computed style was last set. In the context of this bug, it's actually Safari's
     30    AutoFill logic (embedded in the injected bundle) that ends up asking for the computed style of the password
     31    input, forcing it to be created and set (though, as demonstrated in the layout test, simply grabbing the
     32    computed style is sufficient to replicate the bug outside of Safari).
     33   
     34    The end result is that we'll use this stale computed style, which still believes that the password input is not
     35    displayed, and we end up not focusing the element due to believing that the password input is hidden. To fix
     36    this, we would need to either check whether the element has an invalid style (i.e. `needsStyleRecalc()`) before
     37    attempting to use the existing computed style, or clear out the `ElementRareData` computed style anytime the
     38    element's style is invalidated. However, both of these approaches will cause us to perform partial style
     39    resolution much more aggressively, leading to a 2-3% regression in Speedometer.
     40   
     41    To address the bug without hampering our performance wins from r257839, we add a new node flag so that we can
     42    remember when computed styles are no longer valid due to style invalidation, and consult this flag in
     43    `Element::isVisibleWithoutResolvingFullStyle` to avoid using the existing computed style.
     44   
     45    Test: fast/forms/programmatic-focus-after-display.html
     46   
     47    * dom/Element.cpp:
     48    (WebCore::Element::invalidateStyle):
     49    (WebCore::Element::resolveComputedStyle):
     50    (WebCore::Element::isVisibleWithoutResolvingFullStyle const):
     51    * dom/Node.h:
     52    (WebCore::Node::setHasValidStyle):
     53   
     54    LayoutTests:
     55   
     56    Add a new layout test to verify that the bug does not occur. See WebCore/ChangeLog for more details.
     57   
     58    * fast/forms/programmatic-focus-after-display-expected.txt: Added.
     59    * fast/forms/programmatic-focus-after-display.html: Added.
     60   
     61    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266887 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     62
     63    2020-09-10  Wenson Hsieh  <wenson_hsieh@apple.com>
     64
     65            REGRESSION (r257839): clickpay.com - password placeholder text cannot be replaced
     66            https://bugs.webkit.org/show_bug.cgi?id=216257
     67            <rdar://problem/68150686>
     68
     69            Reviewed by Antti Koivisto.
     70
     71            On clickpay.com, the field in the login form that contains the text "Password" is actually a plain text input,
     72            referred to by the site's script as the "null text input". The page adds a focus event listener to this null
     73            text input, and inside of this focus event listener, it reveals a hidden password field by removing an inline
     74            `display: none;` style rule on the real password input element, programmatically focuses it, and then hides the
     75            null text input by setting it to `display: none;` via inline style.
     76
     77            However, after the changes in r257839, we no longer attempt to do a style update upon programmatic focus in the
     78            case where the programmatically focused element does not have a renderer yet (this applies to the password field
     79            in this scenario, because it previously had `display: none;`). When we determine whether the newly displayed
     80            password field is focusable using `Element::isVisibleWithoutResolvingFullStyle`, we then attempt to use either
     81            the existing computed `RenderStyle` on the element, or perform a partial computed style resolution using the
     82            `ResolveComputedStyleMode::RenderedOnly` flag.
     83
     84            But in the case where `ElementRareData`'s computed style exists, it is not guaranteed to be up to date if the
     85            inline style changed since the computed style was last set. In the context of this bug, it's actually Safari's
     86            AutoFill logic (embedded in the injected bundle) that ends up asking for the computed style of the password
     87            input, forcing it to be created and set (though, as demonstrated in the layout test, simply grabbing the
     88            computed style is sufficient to replicate the bug outside of Safari).
     89
     90            The end result is that we'll use this stale computed style, which still believes that the password input is not
     91            displayed, and we end up not focusing the element due to believing that the password input is hidden. To fix
     92            this, we would need to either check whether the element has an invalid style (i.e. `needsStyleRecalc()`) before
     93            attempting to use the existing computed style, or clear out the `ElementRareData` computed style anytime the
     94            element's style is invalidated. However, both of these approaches will cause us to perform partial style
     95            resolution much more aggressively, leading to a 2-3% regression in Speedometer.
     96
     97            To address the bug without hampering our performance wins from r257839, we add a new node flag so that we can
     98            remember when computed styles are no longer valid due to style invalidation, and consult this flag in
     99            `Element::isVisibleWithoutResolvingFullStyle` to avoid using the existing computed style.
     100
     101            Test: fast/forms/programmatic-focus-after-display.html
     102
     103            * dom/Element.cpp:
     104            (WebCore::Element::invalidateStyle):
     105            (WebCore::Element::resolveComputedStyle):
     106            (WebCore::Element::isVisibleWithoutResolvingFullStyle const):
     107            * dom/Node.h:
     108            (WebCore::Node::setHasValidStyle):
     109
    11102020-09-28  Alan Coon  <alancoon@apple.com>
    2111

  • branches/safari-610-branch/Source/WebCore/dom/Element.cpp

    r267643 r267721  
    19451945    Node::invalidateStyle(Style::Validity::ElementInvalid);
    19461946    invalidateSiblingsIfNeeded(*this);
     1947
     1948    // FIXME: This flag should be set whenever styles are invalidated while computed styles are present,
     1949    // not just in this codepath.
     1950    if (hasRareData() && elementRareData()->computedStyle())
     1951        setFlag(IsComputedStyleInvalidFlag);
    19471952}
    19481953
    … …  
    33183323{
    33193324    ASSERT(isConnected());
    3320     ASSERT(!existingComputedStyle());
     3325    ASSERT(!existingComputedStyle() || hasFlag(IsComputedStyleInvalidFlag));
    33213326
    33223327    Deque<RefPtr<Element>, 32> elementsRequiringComputedStyle({ this });
    … …  
    33383343        ElementRareData& rareData = element->ensureElementRareData();
    33393344        rareData.setComputedStyle(WTFMove(style));
     3345        element->clearFlag(IsComputedStyleInvalidFlag);
    33403346
    33413347        if (mode == ResolveComputedStyleMode::RenderedOnly && computedStyle->display() == DisplayType::None)
    … …  
    33673373
    33683374    // Compute style in yet unstyled subtree.
    3369     auto* style = existingComputedStyle();
     3375    auto* style = hasFlag(IsComputedStyleInvalidFlag) ? nullptr : existingComputedStyle();
    33703376    if (!style)
    33713377        style = const_cast<Element&>(*this).resolveComputedStyle(ResolveComputedStyleMode::RenderedOnly);

  • branches/safari-610-branch/Source/WebCore/dom/Node.h

    r267643 r267721  
    554554        ChildrenAffectedByFirstChildRulesFlag = 1 << 25,
    555555        ChildrenAffectedByLastChildRulesFlag = 1 << 26,
    556         // UnusedFlag = 1 << 27,
     556        IsComputedStyleInvalidFlag = 1 << 27,
    557557
    558558        AffectsNextSiblingElementStyle = 1 << 28,
    … …  
    776776    m_nodeFlags &= ~StyleValidityMask;
    777777    clearFlag(StyleResolutionShouldRecompositeLayerFlag);
     778    clearFlag(IsComputedStyleInvalidFlag);
    778779}
    779780
Note: See TracChangeset for help on using the changeset viewer.