Skip to content
Permalink
Browse files
REGRESSION(r274812): Release assert in Document::updateLayout() after…
… calling focus({preventScroll: true}) on a textarea

https://bugs.webkit.org/show_bug.cgi?id=224262

Reviewed by Antti Koivisto.

Source/WebCore:

The regression was caused by Element::focus not updating the selection when preventScroll is set to true.
Fixed it by always updating the selection whenever Element::focus is called.

Test: fast/forms/textarea/textarea-focus-prevent-scroll-crash.html

* dom/Element.cpp:
(WebCore::Element::focus):
(WebCore::Element::findTargetAndUpdateFocusAppearance): Renamed from revealFocusedElement.
* dom/Element.h:
* page/EventHandler.cpp:
(WebCore::EventHandler::dispatchMouseEvent):

LayoutTests:

Added a regression test.

* fast/forms/textarea/textarea-focus-prevent-scroll-crash-expected.txt: Added.
* fast/forms/textarea/textarea-focus-prevent-scroll-crash.html: Added.


Canonical link: https://commits.webkit.org/236236@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275591 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed Apr 7, 2021
1 parent 175fee7 commit 75f647c1341b03ed649fad434cdd529e5973c778
Showing 7 changed files with 65 additions and 8 deletions.
@@ -1,3 +1,15 @@
2021-04-07 Ryosuke Niwa <rniwa@webkit.org>

REGRESSION(r274812): Release assert in Document::updateLayout() after calling focus({preventScroll: true}) on a textarea
https://bugs.webkit.org/show_bug.cgi?id=224262

Reviewed by Antti Koivisto.

Added a regression test.

* fast/forms/textarea/textarea-focus-prevent-scroll-crash-expected.txt: Added.
* fast/forms/textarea/textarea-focus-prevent-scroll-crash.html: Added.

2021-04-06 Sihui Liu <sihui_liu@apple.com>

[ Catalina WK2 Release ] http/tests/IndexedDB/storage-limit-1.https.html is a flaky failure
@@ -0,0 +1,4 @@
This tests calling focus({preventScroll: true}) and removing textarea's children.
WebKit should not crash or hit any assertions.

PASS
@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html>
<body>
<p>This tests calling focus({preventScroll: true}) and removing textarea's children.<br>
WebKit should not crash or hit any assertions.</p>
<script>

if (window.testRunner)
testRunner.dumpAsText();

const input = document.createElement('input');
document.body.appendChild(input);

const textarea = document.createElement('textarea');
input.appendChild(textarea);

textarea.focus({preventScroll: true});
document.body.offsetTop;
textarea.replaceChildren();

document.write('PASS');

</script>
</body>
</html>
@@ -1,3 +1,22 @@
2021-04-07 Ryosuke Niwa <rniwa@webkit.org>

REGRESSION(r274812): Release assert in Document::updateLayout() after calling focus({preventScroll: true}) on a textarea
https://bugs.webkit.org/show_bug.cgi?id=224262

Reviewed by Antti Koivisto.

The regression was caused by Element::focus not updating the selection when preventScroll is set to true.
Fixed it by always updating the selection whenever Element::focus is called.

Test: fast/forms/textarea/textarea-focus-prevent-scroll-crash.html

* dom/Element.cpp:
(WebCore::Element::focus):
(WebCore::Element::findTargetAndUpdateFocusAppearance): Renamed from revealFocusedElement.
* dom/Element.h:
* page/EventHandler.cpp:
(WebCore::EventHandler::dispatchMouseEvent):

2021-04-07 Stephan Szabo <stephan.szabo@sony.com>

[PlayStation] Provide a non-empty User Agent
@@ -3088,19 +3088,16 @@ void Element::focus(const FocusOptions& options)
return;
}

if (!options.preventScroll)
newTarget->revealFocusedElement(options.selectionRestorationMode);
newTarget->findTargetAndUpdateFocusAppearance(options.selectionRestorationMode, options.preventScroll ? SelectionRevealMode::DoNotReveal : SelectionRevealMode::Reveal);
}

void Element::revealFocusedElement(SelectionRestorationMode selectionMode)
void Element::findTargetAndUpdateFocusAppearance(SelectionRestorationMode selectionMode, SelectionRevealMode revealMode)
{
SelectionRevealMode revealMode = SelectionRevealMode::Reveal;

#if PLATFORM(IOS_FAMILY)
// Focusing a form element triggers animation in UIKit to scroll to the right position.
// Calling updateFocusAppearance() would generate an unnecessary call to ScrollView::setScrollPosition(),
// which would jump us around during this animation. See <rdar://problem/6699741>.
if (is<HTMLFormControlElement>(*this))
if (revealMode == SelectionRevealMode::Reveal && is<HTMLFormControlElement>(*this))
revealMode = SelectionRevealMode::RevealUpToMainFrame;
#endif

@@ -405,7 +405,7 @@ class Element : public ContainerNode {

static AXTextStateChangeIntent defaultFocusTextStateChangeIntent() { return AXTextStateChangeIntent(AXTextStateChangeTypeSelectionMove, AXTextSelection { AXTextSelectionDirectionDiscontiguous, AXTextSelectionGranularityUnknown, true }); }
virtual void focus(const FocusOptions& = { });
void revealFocusedElement(SelectionRestorationMode);
void findTargetAndUpdateFocusAppearance(SelectionRestorationMode, SelectionRevealMode = SelectionRevealMode::Reveal);
virtual RefPtr<Element> focusAppearanceUpdateTarget();
virtual void updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode = SelectionRevealMode::Reveal);
virtual void blur();
@@ -2723,7 +2723,7 @@ bool EventHandler::dispatchMouseEvent(const AtomString& eventType, Node* targetN
return false;

if (element && m_mouseDownDelegatedFocus)
element->revealFocusedElement(SelectionRestorationMode::SelectAll);
element->findTargetAndUpdateFocusAppearance(SelectionRestorationMode::SelectAll);

return true;
}

0 comments on commit 75f647c

Please sign in to comment.