Bug 242181 - Simplify resolveForDocument
Summary: Simplify resolveForDocument
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-06-30 05:57 PDT by Rob Buis
Modified: 2022-07-04 00:45 PDT (History)
8 users (show)

See Also:

Attachments
Patch (1.48 KB, patch)
2022-06-30 06:07 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (3.08 KB, patch)
2022-07-02 12:42 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (3.23 KB, patch)
2022-07-03 11:29 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2022-06-30 05:57:28 PDT 
Simplify resolveForDocument.
Comment 1 Rob Buis 2022-06-30 06:07:13 PDT 
Created attachment 460577 [details]
Patch
Comment 2 Darin Adler 2022-07-01 22:28:45 PDT 
Comment on attachment 460577 [details]
Patch

Where is the explanation for why this code can be removed? Why was the code there in the first place? Why does it turn out to not be needed? It’s great that all the tests pass, so I am guessing this code is truly unnecessary, but how did you determine that? Are you sure this isn’t just due to a missing test case?
Comment 3 Rob Buis 2022-07-02 12:42:40 PDT 
Created attachment 460635 [details]
Patch
Comment 4 Rob Buis 2022-07-03 11:29:23 PDT 
Created attachment 460641 [details]
Patch
Comment 5 Rob Buis 2022-07-03 14:29:44 PDT 
(In reply to Darin Adler from comment #2)
> Comment on attachment 460577 [details]
> Patch
> 
> Where is the explanation for why this code can be removed? Why was the code
> there in the first place? Why does it turn out to not be needed? It’s great
> that all the tests pass, so I am guessing this code is truly unnecessary,
> but how did you determine that? Are you sure this isn’t just due to a
> missing test case?

I found this out just by code inspection. I checked all getters and setters between the two FontCascadeDescription usages but they do not depend on it, so the change should be safe.
Comment 6 EWS 2022-07-04 00:44:56 PDT 
Committed 252111@main (d2b5507f6b8e): <https://commits.webkit.org/252111@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 460641 [details].
Comment 7 Radar WebKit Bug Importer 2022-07-04 00:45:14 PDT 
<rdar://problem/96384588>