Bug 276677 - [REGRESSION] LocalDOMWindow::getMatchedCSSRules asserts when passed null/empty pseudoElement
Summary: [REGRESSION] LocalDOMWindow::getMatchedCSSRules asserts when passed null/empt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 18
Hardware: All All
: P2 Major
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2024-07-16 13:28 PDT by Jonathan Hammer
Modified: 2024-07-19 10:26 PDT (History)
2 users (show)

See Also:


Attachments
Patch to fix LocalDOMWindow::getMatchedCSSRules (755 bytes, patch)
2024-07-16 13:31 PDT, Jonathan Hammer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Hammer 2024-07-16 13:28:50 PDT
In debug builds, calling LocalDOMWindow::getMatchedCSSRules with a null or empty pseudoElement parameter results in an assertion failure in PseudoElementRequest(). In release builds, getMatchedCSSRules incorrectly returns null. This API, while deprecated, is still exposed to Obj-C clients via -[DOMDocument getMatchedCSSRules:pseudoElement:authorOnly:], and our macOS application [1] relies on it working correctly. Our app is currently broken on the macOS Sequoia beta builds due to this bug.

It looks like this is the commit that inadvertently caused the issue:

https://github.com/WebKit/WebKit/commit/4d69396f11340703fd2c7cffc87e2ed11b26633f

In that commit, two changes combined to create the bug:

(1) The signature for Resolver::pseudoStyleRulesForElement changed (the pseudoId parameter is now a std::optional<PseudoId> instead of PseudoId)
(2) The PseudoElementRequest constructor was modified to assert pseudoId != PseudoId::None

When LocalDOMWindow::getMatchedCSSRules is called with an empty pseudoElement parameter, it ends up calling pseudoStyleRulesForElement with PseudoId::None, which triggers the assertion. 

Proposed Fix:

The method getMatchedCSSRules should be changed to call pseudoStyleRulesForElement with std::nullopt instead of PseudoId::None.

LocalDOMWindow.cpp:1636:

-    auto pseudoId = pseudoElementIdentifier ? pseudoElementIdentifier->pseudoId : PseudoId::None;
+    std::optional<PseudoId> pseudoId;
+    if (pseudoElementIdentifier)
+        pseudoId = pseudoElementIdentifier->pseudoId;

Thanks,
Jonathan

[1]: https://directmailmac.com
Comment 1 Jonathan Hammer 2024-07-16 13:31:11 PDT
Created attachment 471896 [details]
Patch to fix LocalDOMWindow::getMatchedCSSRules
Comment 2 Jonathan Hammer 2024-07-17 07:59:58 PDT
Pull request: https://github.com/WebKit/WebKit/pull/30906
Comment 3 EWS 2024-07-18 07:44:54 PDT
Committed 281088@main (9b380bdb5e47): <https://commits.webkit.org/281088@main>

Reviewed commits have been landed. Closing PR #30906 and removing active labels.
Comment 4 Radar WebKit Bug Importer 2024-07-18 07:45:18 PDT
<rdar://problem/132009080>
Comment 5 EWS 2024-07-19 10:26:30 PDT
Committed 280938.67@integration/ci/132009080_9b380bdb5e_safari-7619-branch (372659bf557d): <https://commits.webkit.org/280938.67@integration/ci/132009080_9b380bdb5e_safari-7619-branch>

Reviewed commits have been landed. Closing PR #1433 and removing active labels.