Bug 242526 - :focus-within pseudo class doesn't get invalidated when frame loses focus
Summary: :focus-within pseudo class doesn't get invalidated when frame loses focus
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-07-08 10:46 PDT by Ryosuke Niwa
Modified: 2022-07-09 20:41 PDT (History)
13 users (show)

See Also:


Attachments
test case (930 bytes, text/html)
2022-07-08 10:46 PDT, Ryosuke Niwa
no flags Details
WIP (2.40 KB, patch)
2022-07-08 10:48 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
WIP2 (4.89 KB, patch)
2022-07-09 00:24 PDT, Ryosuke Niwa
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2022-07-08 10:46:42 PDT
Created attachment 460764 [details]
test case

Reproduction steps:
1. Open the attached document in Safari / Mini browser
2. Focus the address bar / other parts of app

Expected result:
There will be 100px by 100px green box

Actual result:
Green box remains 50px by 50px
Comment 1 Ryosuke Niwa 2022-07-08 10:48:31 PDT
Created attachment 460765 [details]
WIP

This patch doesn't quite work.

Replacing
Style::PseudoClassChangeInvalidation focusStyleInvalidation(lineage, { { CSSSelector::PseudoClassFocusWithin, activeAndFocused } });
with
lineage.invalidateStyle();
makes it work.
Comment 2 Ryosuke Niwa 2022-07-08 23:53:21 PDT
So :focus, :focus-within, :focus-visible all depend on FrameSelection::isFocusedAndActive() via isFrameFocused in SelectorCheckerTestFunctions.h

FrameSelection::isFocusedAndActive() in turn is defined in terms of two boolean states: FrameSelection::m_focused and FocusController::isActive().

FocusController calls FrameSelection::pageActivationChanged() whenever isActive state changes on the focused frame's FrameSelection.

In the ideal world, we have a single place where these two boolean states are updated at once, and that's where we instantiate relevant Style::PseudoClassChangeInvalidation but that's tricky because FocusController is a per-Page concept and FrameSelection is per-Frame concept.

To reconcile, I could imagine a few solutions:
1. Add a new boolean m_isPageActive to FrameSelection which gets sync'ed when FrameSelection becomes newly focused (inside FrameSelection::setFocused). We then update both m_focused & this new boolean whilst we have Style::PseudoClassChangeInvalidation.

2. In FrameSelection::focusedOrActiveStateChanged(), introduce a temporary override for FrameSelection::isFocusedAndActive(), which makes the function return the negative value while Style::PseudoClassChangeInvalidation are instantiated.

3. Simply use Element::invalidateStyle() on these elements for now. This will only occur when WKWebView becomes first responder or resigns first responder (i.e. view gets focused or unfocused in more Web / Windows terminology).
Comment 3 Ryosuke Niwa 2022-07-09 00:24:54 PDT
Created attachment 460776 [details]
WIP2
Comment 4 Ryosuke Niwa 2022-07-09 00:56:39 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2259
Comment 5 EWS 2022-07-09 20:40:50 PDT
Committed 252324@main (ac8282feee7f): <https://commits.webkit.org/252324@main>

Reviewed commits have been landed. Closing PR #2259 and removing active labels.
Comment 6 Radar WebKit Bug Importer 2022-07-09 20:41:18 PDT
<rdar://problem/96786471>