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

Changeset 267631 in webkit


Ignore:
Timestamp:
Sep 26, 2020, 2:08:38 PM (6 years ago)
Author:
Alan Coon
Message:

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

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

    r267629 r267631  
     12020-09-25  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-25  Alan Coon  <alancoon@apple.com>
    275

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

    r267630 r267631  
     12020-09-25  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            On clickpay.com, the field in the login form that contains the text "Password" is actually a plain text input,
     70            referred to by the site's script as the "null text input". The page adds a focus event listener to this null
     71            text input, and inside of this focus event listener, it reveals a hidden password field by removing an inline
     72            `display: none;` style rule on the real password input element, programmatically focuses it, and then hides the
     73            null text input by setting it to `display: none;` via inline style.
     74
     75            However, after the changes in r257839, we no longer attempt to do a style update upon programmatic focus in the
     76            case where the programmatically focused element does not have a renderer yet (this applies to the password field
     77            in this scenario, because it previously had `display: none;`). When we determine whether the newly displayed
     78            password field is focusable using `Element::isVisibleWithoutResolvingFullStyle`, we then attempt to use either
     79            the existing computed `RenderStyle` on the element, or perform a partial computed style resolution using the
     80            `ResolveComputedStyleMode::RenderedOnly` flag.
     81
     82            But in the case where `ElementRareData`'s computed style exists, it is not guaranteed to be up to date if the
     83            inline style changed since the computed style was last set. In the context of this bug, it's actually Safari's
     84            AutoFill logic (embedded in the injected bundle) that ends up asking for the computed style of the password
     85            input, forcing it to be created and set (though, as demonstrated in the layout test, simply grabbing the
     86            computed style is sufficient to replicate the bug outside of Safari).
     87
     88            The end result is that we'll use this stale computed style, which still believes that the password input is not
     89            displayed, and we end up not focusing the element due to believing that the password input is hidden. To fix
     90            this, we would need to either check whether the element has an invalid style (i.e. `needsStyleRecalc()`) before
     91            attempting to use the existing computed style, or clear out the `ElementRareData` computed style anytime the
     92            element's style is invalidated. However, both of these approaches will cause us to perform partial style
     93            resolution much more aggressively, leading to a 2-3% regression in Speedometer.
     94
     95            To address the bug without hampering our performance wins from r257839, we add a new node flag so that we can
     96            remember when computed styles are no longer valid due to style invalidation, and consult this flag in
     97            `Element::isVisibleWithoutResolvingFullStyle` to avoid using the existing computed style.
     98
     99            Test: fast/forms/programmatic-focus-after-display.html
     100
     101            * dom/Element.cpp:
     102            (WebCore::Element::invalidateStyle):
     103            (WebCore::Element::resolveComputedStyle):
     104            (WebCore::Element::isVisibleWithoutResolvingFullStyle const):
     105            * dom/Node.h:
     106            (WebCore::Node::setHasValidStyle):
     107
    11082020-09-25  Alan Coon  <alancoon@apple.com>
    2109

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

    r265820 r267631  
    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        setNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);
    19471952}
    19481953
    … …  
    33183323{
    33193324    ASSERT(isConnected());
    3320     ASSERT(!existingComputedStyle());
     3325    ASSERT(!existingComputedStyle() || hasNodeFlag(NodeFlag::IsComputedStyleInvalidFlag));
    33213326
    33223327    Deque<RefPtr<Element>, 32> elementsRequiringComputedStyle({ this });
    … …  
    33383343        ElementRareData& rareData = element->ensureElementRareData();
    33393344        rareData.setComputedStyle(WTFMove(style));
     3345        element->clearNodeFlag(NodeFlag::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 = hasNodeFlag(NodeFlag::IsComputedStyleInvalidFlag) ? nullptr : existingComputedStyle();
    33703376    if (!style)
    33713377        style = const_cast<Element&>(*this).resolveComputedStyle(ResolveComputedStyleMode::RenderedOnly);

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

    r262695 r267631  
    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    clearNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);
    778779}
    779780
Note: See TracChangeset for help on using the changeset viewer.