Bug 228123 - [iOS] Add SPI for internal clients to consult whether or not viewport quirks should be enabled
Summary: [iOS] Add SPI for internal clients to consult whether or not viewport quirks ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-20 13:47 PDT by Wenson Hsieh
Modified: 2021-07-20 19:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.68 KB, patch)
2021-07-20 16:28 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Renaming (6.68 KB, patch)
2021-07-20 18:18 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Renaming (7.12 KB, patch)
2021-07-20 18:19 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-07-20 13:47:08 PDT
rdar://80397679
Comment 1 Wenson Hsieh 2021-07-20 16:28:12 PDT
Created attachment 433906 [details]
Patch
Comment 2 EWS 2021-07-20 17:51:36 PDT
Committed r280119 (239833@main): <https://commits.webkit.org/239833@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433906 [details].
Comment 3 mitz 2021-07-20 17:57:05 PDT
Can you explain here why clients can’t use the _needsSiteSpecificQuirks property of WKPreferences?
Comment 4 Wenson Hsieh 2021-07-20 18:02:48 PDT
(In reply to mitz from comment #3)
> Can you explain here why clients can’t use the _needsSiteSpecificQuirks
> property of WKPreferences?

The rationale is that this value will change out from underneath the WebKit client, so  we'd want to avoid situations where a client that sets the `_needsSiteSpecificQuirks` property later reads back a different value than what it previously set.

More generally, this is a hint from WebKit to the client, whereas WKPreferences is a way for clients to provide hints to WebKit.

The naming collision is a bit unfortunate, though. I think that ideally, the preference would be called something like `_prefersSiteSpecificQuirks` to indicate a difference between the "hint" and "effective" value.
Comment 5 mitz 2021-07-20 18:05:01 PDT
(In reply to Wenson Hsieh from comment #4)
> (In reply to mitz from comment #3)
> > Can you explain here why clients can’t use the _needsSiteSpecificQuirks
> > property of WKPreferences?
> 
> The rationale is that this value will change out from underneath the WebKit
> client, so  we'd want to avoid situations where a client that sets the
> `_needsSiteSpecificQuirks` property later reads back a different value than
> what it previously set.
> 
> More generally, this is a hint from WebKit to the client, whereas
> WKPreferences is a way for clients to provide hints to WebKit.
> 
> The naming collision is a bit unfortunate, though. I think that ideally, the
> preference would be called something like `_prefersSiteSpecificQuirks` to
> indicate a difference between the "hint" and "effective" value.

Thank you. I think I now understand this new property’s purpose. Seems like adding “viewport” to the name would have helped with both disambiguation and clarifying the intent.
Comment 6 Wenson Hsieh 2021-07-20 18:06:27 PDT
(In reply to mitz from comment #5)
> (In reply to Wenson Hsieh from comment #4)
> > (In reply to mitz from comment #3)
> > > Can you explain here why clients can’t use the _needsSiteSpecificQuirks
> > > property of WKPreferences?
> > 
> > The rationale is that this value will change out from underneath the WebKit
> > client, so  we'd want to avoid situations where a client that sets the
> > `_needsSiteSpecificQuirks` property later reads back a different value than
> > what it previously set.
> > 
> > More generally, this is a hint from WebKit to the client, whereas
> > WKPreferences is a way for clients to provide hints to WebKit.
> > 
> > The naming collision is a bit unfortunate, though. I think that ideally, the
> > preference would be called something like `_prefersSiteSpecificQuirks` to
> > indicate a difference between the "hint" and "effective" value.
> 
> Thank you. I think I now understand this new property’s purpose. Seems like
> adding “viewport” to the name would have helped with both disambiguation and
> clarifying the intent.

The patch adopting this in the WebKit client has not landed yet, so we can (easily) fix this!
Comment 7 Wenson Hsieh 2021-07-20 18:18:24 PDT
Reopening to attach new patch.
Comment 8 Wenson Hsieh 2021-07-20 18:18:25 PDT Comment hidden (obsolete)
Comment 9 Wenson Hsieh 2021-07-20 18:19:19 PDT
Created attachment 433913 [details]
Renaming
Comment 10 mitz 2021-07-20 19:07:11 PDT
Comment on attachment 433913 [details]
Renaming

Thank you!
Comment 11 Wenson Hsieh 2021-07-20 19:37:56 PDT
Comment on attachment 433913 [details]
Renaming

Thanks for the review!
Comment 12 EWS 2021-07-20 19:47:50 PDT
Committed r280122 (239834@main): <https://commits.webkit.org/239834@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433913 [details].