Changeset 191451 in webkit


Ignore:
Timestamp:
Oct 22, 2015, 4:37:46 AM (10 years ago)
Author:
rniwa@webkit.org
Message:

REGRESSION (r181972): Scroll position changes to top of youtube page when switching tabs
​https://bugs.webkit.org/show_bug.cgi?id=150428

Reviewed by Antti Koivisto.

Source/WebCore:

The bug was caused by updateFocusAppearance in WebPage::restoreSelectionInFocusedEditableElement
revealing the focused element which was added in r181972. Fixed the bug by adding an option to
suppress this behavior here.

  • dom/Document.cpp:

(WebCore::Document::Document):
(WebCore::Document::updateFocusAppearanceSoon):

  • dom/Document.h:
  • dom/Element.cpp:

(WebCore::Element::focus):
(WebCore::Element::updateFocusAppearanceAfterAttachIfNeeded):
(WebCore::Element::updateFocusAppearance):

  • dom/Element.h:
  • history/CachedPage.cpp:

(WebCore::CachedPage::restore):

  • html/HTMLAreaElement.cpp:

(WebCore::HTMLAreaElement::updateFocusAppearance):

  • html/HTMLAreaElement.h:
  • html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::updateFocusAppearance):
(WebCore::HTMLInputElement::runPostTypeUpdateTasks):
(WebCore::HTMLInputElement::didAttachRenderers):

  • html/HTMLInputElement.h:
  • html/HTMLTextAreaElement.cpp:

(WebCore::HTMLTextAreaElement::updateFocusAppearance):

  • html/HTMLTextAreaElement.h:

Source/WebKit2:

Call updateFocusAppearance with RevealMode::DoNotReveal to avoid revealing the focused element.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::restoreSelectionInFocusedEditableElement):

Tools:

Added a regression test using WebKit API test.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/mac/FirstResponderScrollingPosition.mm: Added.

(TestWebKitAPI::didFinishLoadForFrame):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
1 added
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r191450 r191451  
     12015-10-22  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (r181972): Scroll position changes to top of youtube page when switching tabs
     4        https://bugs.webkit.org/show_bug.cgi?id=150428
     5
     6        Reviewed by Antti Koivisto.
     7
     8        The bug was caused by updateFocusAppearance in WebPage::restoreSelectionInFocusedEditableElement
     9        revealing the focused element which was added in r181972. Fixed the bug by adding an option to
     10        suppress this behavior here.
     11
     12        * dom/Document.cpp:
     13        (WebCore::Document::Document):
     14        (WebCore::Document::updateFocusAppearanceSoon):
     15        * dom/Document.h:
     16        * dom/Element.cpp:
     17        (WebCore::Element::focus):
     18        (WebCore::Element::updateFocusAppearanceAfterAttachIfNeeded):
     19        (WebCore::Element::updateFocusAppearance):
     20        * dom/Element.h:
     21        * history/CachedPage.cpp:
     22        (WebCore::CachedPage::restore):
     23        * html/HTMLAreaElement.cpp:
     24        (WebCore::HTMLAreaElement::updateFocusAppearance):
     25        * html/HTMLAreaElement.h:
     26        * html/HTMLInputElement.cpp:
     27        (WebCore::HTMLInputElement::updateFocusAppearance):
     28        (WebCore::HTMLInputElement::runPostTypeUpdateTasks):
     29        (WebCore::HTMLInputElement::didAttachRenderers):
     30        * html/HTMLInputElement.h:
     31        * html/HTMLTextAreaElement.cpp:
     32        (WebCore::HTMLTextAreaElement::updateFocusAppearance):
     33        * html/HTMLTextAreaElement.h:
     34
    1352015-10-22  Joonghun Park  <jh718.park@samsung.com>
    236

  • trunk/Source/WebCore/dom/Document.cpp

    r191262 r191451  
    456456    , m_gotoAnchorNeededAfterStylesheetsLoad(false)
    457457    , m_frameElementsShouldIgnoreScrolling(false)
    458     , m_updateFocusAppearanceRestoresSelection(false)
     458    , m_updateFocusAppearanceRestoresSelection(SelectionRestorationMode::SetDefault)
    459459    , m_ignoreDestructiveWriteCount(0)
    460460    , m_markers(std::make_unique<DocumentMarkerController>(*this))
    … …  
    50805080}
    50815081
    5082 void Document::updateFocusAppearanceSoon(bool restorePreviousSelection)
    5083 {
    5084     m_updateFocusAppearanceRestoresSelection = restorePreviousSelection;
     5082void Document::updateFocusAppearanceSoon(SelectionRestorationMode mode)
     5083{
     5084    m_updateFocusAppearanceRestoresSelection = mode;
    50855085    if (!m_updateFocusAppearanceTimer.isActive())
    50865086        m_updateFocusAppearanceTimer.startOneShot(0);

  • trunk/Source/WebCore/dom/Document.h

    r191262 r191451  
    275275enum DimensionsCheck { WidthDimensionsCheck = 1 << 0, HeightDimensionsCheck = 1 << 1, AllDimensionsCheck = 1 << 2 };
    276276
     277enum class SelectionRestorationMode {
     278    Restore,
     279    SetDefault,
     280};
     281
     282enum class SelectionRevealMode {
     283    Reveal,
     284    DoNotReveal
     285};
     286
    277287enum class HttpEquivPolicy {
    278288    Enabled,
    … …  
    967977    void setHasNodesWithPlaceholderStyle() { m_hasNodesWithPlaceholderStyle = true; }
    968978
    969     void updateFocusAppearanceSoon(bool restorePreviousSelection);
     979    void updateFocusAppearanceSoon(SelectionRestorationMode);
    970980    void cancelFocusAppearanceUpdate();
    971981
    … …  
    14931503    bool m_haveExplicitlyDisabledDNSPrefetch;
    14941504    bool m_frameElementsShouldIgnoreScrolling;
    1495     bool m_updateFocusAppearanceRestoresSelection;
     1505    SelectionRestorationMode m_updateFocusAppearanceRestoresSelection;
    14961506
    14971507    // http://www.whatwg.org/specs/web-apps/current-work/#ignore-destructive-writes-counter

  • trunk/Source/WebCore/dom/Element.cpp

    r191262 r191451  
    22202220        view->setProhibitsScrolling(true);
    22212221#endif
    2222     updateFocusAppearance(restorePreviousSelection);
     2222    updateFocusAppearance(restorePreviousSelection ? SelectionRestorationMode::Restore : SelectionRestorationMode::SetDefault);
    22232223#if PLATFORM(IOS)
    22242224    if (isFormControl)
    … …  
    22352235        return;
    22362236    if (isFocusable() && document().focusedElement() == this)
    2237         document().updateFocusAppearanceSoon(false /* don't restore selection */);
     2237        document().updateFocusAppearanceSoon(SelectionRestorationMode::SetDefault);
    22382238    data->setNeedsFocusAppearanceUpdateSoonAfterAttach(false);
    22392239}
    22402240
    2241 void Element::updateFocusAppearance(bool /*restorePreviousSelection*/)
     2241void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode revealMode)
    22422242{
    22432243    if (isRootEditableElement()) {
    … …  
    22552255        if (frame->selection().shouldChangeSelection(newSelection)) {
    22562256            frame->selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(), Element::defaultFocusTextStateChangeIntent());
    2257             frame->selection().revealSelection();
     2257            if (revealMode == SelectionRevealMode::Reveal)
     2258                frame->selection().revealSelection();
    22582259        }
    2259     } else if (renderer() && !renderer()->isWidget())
     2260    } else if (renderer() && !renderer()->isWidget() && revealMode == SelectionRevealMode::Reveal)
    22602261        renderer()->scrollRectToVisible(renderer()->anchorRect());
    22612262}

  • trunk/Source/WebCore/dom/Element.h

    r191262 r191451  
    328328    void updateFocusAppearanceAfterAttachIfNeeded();
    329329    virtual void focus(bool restorePreviousSelection = true, FocusDirection = FocusDirectionNone);
    330     virtual void updateFocusAppearance(bool restorePreviousSelection);
     330    virtual void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode = SelectionRevealMode::Reveal);
    331331    virtual void blur();
    332332

  • trunk/Source/WebCore/history/CachedPage.cpp

    r184304 r191451  
    9393        }
    9494#endif
    95         element->updateFocusAppearance(true);
     95        element->updateFocusAppearance(SelectionRestorationMode::Restore);
    9696#if PLATFORM(IOS)
    9797        if (frameView)

  • trunk/Source/WebCore/html/HTMLAreaElement.cpp

    r182120 r191451  
    225225}
    226226   
    227 void HTMLAreaElement::updateFocusAppearance(bool restorePreviousSelection)
     227void HTMLAreaElement::updateFocusAppearance(SelectionRestorationMode restorationMode, SelectionRevealMode revealMode)
    228228{
    229229    if (!isFocusable())
    … …  
    234234        return;
    235235
    236     imageElement->updateFocusAppearance(restorePreviousSelection);
     236    imageElement->updateFocusAppearance(restorationMode, revealMode);
    237237}
    238238   

  • trunk/Source/WebCore/html/HTMLAreaElement.h

    r177996 r191451  
    5858    virtual bool isMouseFocusable() const override;
    5959    virtual bool isFocusable() const override;
    60     virtual void updateFocusAppearance(bool /*restorePreviousSelection*/) override;
     60    virtual void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode) override;
    6161    virtual void setFocus(bool) override;
    6262

  • trunk/Source/WebCore/html/HTMLInputElement.cpp

    r190845 r191451  
    403403}
    404404
    405 void HTMLInputElement::updateFocusAppearance(bool restorePreviousSelection)
     405void HTMLInputElement::updateFocusAppearance(SelectionRestorationMode restorationMode, SelectionRevealMode revealMode)
    406406{
    407407    if (isTextField()) {
    408         if (!restorePreviousSelection || !hasCachedSelection())
     408        if (restorationMode == SelectionRestorationMode::SetDefault || !hasCachedSelection())
    409409            select(Element::defaultFocusTextStateChangeIntent());
    410410        else
    411411            restoreCachedSelection();
    412         if (document().frame())
     412        if (document().frame() && revealMode == SelectionRevealMode::Reveal)
    413413            document().frame()->selection().revealSelection();
    414414    } else
    415         HTMLTextFormControlElement::updateFocusAppearance(restorePreviousSelection);
     415        HTMLTextFormControlElement::updateFocusAppearance(restorationMode, revealMode);
    416416}
    417417
    … …  
    529529
    530530    if (document().focusedElement() == this)
    531         updateFocusAppearance(true);
     531        updateFocusAppearance(SelectionRestorationMode::Restore, SelectionRevealMode::Reveal);
    532532
    533533    setChangedSinceLastFormControlChangeEvent(false);
    … …  
    786786
    787787    if (document().focusedElement() == this)
    788         document().updateFocusAppearanceSoon(true /* restore selection */);
     788        document().updateFocusAppearanceSoon(SelectionRestorationMode::Restore);
    789789}
    790790

  • trunk/Source/WebCore/html/HTMLInputElement.h

    r191327 r191451  
    344344    virtual bool isEnumeratable() const override final;
    345345    virtual bool supportLabels() const override final;
    346     virtual void updateFocusAppearance(bool restorePreviousSelection) override final;
     346    virtual void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode) override final;
    347347    virtual bool shouldUseInputMethod() override final;
    348348

  • trunk/Source/WebCore/html/HTMLTextAreaElement.cpp

    r190613 r191451  
    252252}
    253253
    254 void HTMLTextAreaElement::updateFocusAppearance(bool restorePreviousSelection)
    255 {
    256     if (!restorePreviousSelection || !hasCachedSelection()) {
     254void HTMLTextAreaElement::updateFocusAppearance(SelectionRestorationMode restorationMode, SelectionRevealMode revealMode)
     255{
     256    if (restorationMode == SelectionRestorationMode::SetDefault || !hasCachedSelection()) {
    257257        // If this is the first focus, set a caret at the beginning of the text. 
    258258        // This matches some browsers' behavior; see bug 11746 Comment #15.
    … …  
    262262        restoreCachedSelection(Element::defaultFocusTextStateChangeIntent());
    263263
    264     if (document().frame())
     264    if (document().frame() && revealMode == SelectionRevealMode::Reveal)
    265265        document().frame()->selection().revealSelection();
    266266}

  • trunk/Source/WebCore/html/HTMLTextAreaElement.h

    r189841 r191451  
    109109    virtual bool isMouseFocusable() const override;
    110110    virtual bool isKeyboardFocusable(KeyboardEvent*) const override;
    111     virtual void updateFocusAppearance(bool restorePreviousSelection) override;
     111    virtual void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode) override;
    112112
    113113    virtual void accessKeyAction(bool sendMouseEvents) override;

  • trunk/Source/WebKit2/ChangeLog

    r191449 r191451  
     12015-10-22  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (r181972): Scroll position changes to top of youtube page when switching tabs
     4        https://bugs.webkit.org/show_bug.cgi?id=150428
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Call updateFocusAppearance with RevealMode::DoNotReveal to avoid revealing the focused element.
     9
     10        * WebProcess/WebPage/WebPage.cpp:
     11        (WebKit::WebPage::restoreSelectionInFocusedEditableElement):
     12
    1132015-10-22  Carlos Garcia Campos  <cgarcia@igalia.com>
    214

  • trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

    r191408 r191451  
    34453445    if (auto document = frame.document()) {
    34463446        if (auto element = document->focusedElement())
    3447             element->updateFocusAppearance(true /* restoreSelection */);
     3447            element->updateFocusAppearance(SelectionRestorationMode::Restore, SelectionRevealMode::DoNotReveal);
    34483448    }
    34493449}

  • trunk/Tools/ChangeLog

    r191448 r191451  
     12015-10-22  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION (r181972): Scroll position changes to top of youtube page when switching tabs
     4        https://bugs.webkit.org/show_bug.cgi?id=150428
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Added a regression test using WebKit API test.
     9
     10        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     11        * TestWebKitAPI/Tests/mac/FirstResponderScrollingPosition.mm: Added.
     12        (TestWebKitAPI::didFinishLoadForFrame):
     13        (TestWebKitAPI::TEST):
     14
    1152015-10-22  Carlos Garcia Campos  <cgarcia@igalia.com>
    216

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r191183 r191451  
    266266                9B26FCCA159D16DE00CC3765 /* HTMLFormCollectionNamedItem.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B26FCB4159D15E700CC3765 /* HTMLFormCollectionNamedItem.html */; };
    267267                9B4F8FA7159D52DD002D9F94 /* HTMLCollectionNamedItem.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B4F8FA6159D52CA002D9F94 /* HTMLCollectionNamedItem.html */; };
     268                9B7916501BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9B79164F1BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm */; settings = {ASSET_TAGS = (); }; };
    268269                A13EBBAA1B87428D00097110 /* WebProcessPlugIn.mm in Sources */ = {isa = PBXBuildFile; fileRef = A13EBBA91B87428D00097110 /* WebProcessPlugIn.mm */; };
    269270                A13EBBAB1B87434600097110 /* PlatformUtilitiesCocoa.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0F139E721A423A2B00F590F5 /* PlatformUtilitiesCocoa.mm */; };
    … …  
    639640                9B4F8FA3159D52B1002D9F94 /* HTMLCollectionNamedItem.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = HTMLCollectionNamedItem.mm; sourceTree = "<group>"; };
    640641                9B4F8FA6159D52CA002D9F94 /* HTMLCollectionNamedItem.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = HTMLCollectionNamedItem.html; sourceTree = "<group>"; };
     642                9B79164F1BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FirstResponderScrollingPosition.mm; sourceTree = "<group>"; };
    641643                A13EBB491B87339E00097110 /* TestWebKitAPI.wkbundle */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = TestWebKitAPI.wkbundle; sourceTree = BUILT_PRODUCTS_DIR; };
    642644                A13EBB521B87346600097110 /* WebProcessPlugIn.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = WebProcessPlugIn.xcconfig; sourceTree = "<group>"; };
    … …  
    13141316                                E19DB9781B32137C00DB38D4 /* NavigatorLanguage.mm */,
    13151317                                A57A34EF16AF677200C2501F /* PageVisibilityStateWithWindowChanges.mm */,
     1318                                9B79164F1BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm */,
    13161319                                00BC16851680FE810065F1E5 /* PublicSuffix.mm */,
    13171320                                37C784DE197C8F2E0010A496 /* RenderedImageFromDOMNode.mm */,
    … …  
    17531756                                7A5623111AD5AF3E0096B920 /* MenuTypesForMouseEvents.cpp in Sources */,
    17541757                                51CB4AD81B3A079C00C1B1C6 /* ModalAlertsSPI.cpp in Sources */,
     1758                                9B7916501BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm in Sources */,
    17551759                                1CF0D3791BBF2F3D00B4EF54 /* WKRetainPtr.cpp in Sources */,
    17561760                                26F6E1F01ADC749B00DE696B /* DFAMinimizer.cpp in Sources */,
Note: See TracChangeset for help on using the changeset viewer.