Bug 210772 - Set _STAttributionDisplayName to iOS WebContent Info plist
Summary: Set _STAttributionDisplayName to iOS WebContent Info plist
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 211172
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-20 16:25 PDT by Luming Yin
Modified: 2020-04-29 10:20 PDT (History)
14 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2020-04-20 16:29 PDT, Luming Yin
no flags Details | Formatted Diff | Diff
Patch (7.94 KB, patch)
2020-04-20 17:01 PDT, Luming Yin
no flags Details | Formatted Diff | Diff
Patch (18.45 KB, patch)
2020-04-24 04:56 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (20.40 KB, patch)
2020-04-24 05:43 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (21.47 KB, patch)
2020-04-24 05:45 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (20.44 KB, patch)
2020-04-24 06:55 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (1.52 KB, patch)
2020-04-29 02:19 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luming Yin 2020-04-20 16:25:48 PDT
<rdar://problem/62075201>
Comment 1 Luming Yin 2020-04-20 16:29:33 PDT
Created attachment 397036 [details]
Patch
Comment 2 Luming Yin 2020-04-20 17:01:47 PDT
Created attachment 397039 [details]
Patch
Comment 3 Alexey Proskuryakov 2020-04-20 17:36:22 PDT
Comment on attachment 397039 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397039&action=review

> Source/WebKit/ChangeLog:10
> +        Give the WebContent XPC service bundle a localizable CFBundleDisplayName to make it
> +        more easily identifiable.

WebContent sets its name dynamically, that's how it uses webpage domain name for Safari, or includes "Mail" in the name for Mail. 

Adding a static way to do the same appears confusing and thus undesirable.
Comment 4 Luming Yin 2020-04-20 18:07:31 PDT
(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 397039 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397039&action=review
> 
> > Source/WebKit/ChangeLog:10
> > +        Give the WebContent XPC service bundle a localizable CFBundleDisplayName to make it
> > +        more easily identifiable.
> 
> WebContent sets its name dynamically, that's how it uses webpage domain name
> for Safari, or includes "Mail" in the name for Mail. 
> 
> Adding a static way to do the same appears confusing and thus undesirable.


Some frameworks are only able to read names statically declared in the Info.plist 
of an executable (or XPC binary in this case). Since these frameworks are unable 
to retrieve the dynamic name nonetheless, wouldn't it make sense to at least have 
a reasonable fallback attribution to avoid user confusion?
Comment 5 Alexey Proskuryakov 2020-04-20 18:41:26 PDT
This depends on how the default is used. What seems like a reasonable fallback in one scenario can end up confusing users or being a security problem in another scenario. Obviously, not everything displayed by WebContent is a website.

On the technical level, I'm not sure about the implications:

1. How does CFBundleDisplayName interact with _kLSDisplayNameKey. Looks like we may get away for now with using one on macOS and another on iOS (but unsure what macCatalyst should do). But that seems fragile.

2. What else is CFBundleDisplayName used for. How bad is it to get "Website" as the name in those situations?

Documentation says "The user-visible name for the bundle, used by Siri and visible on the iOS Home screen", and "Use this key if you want a product name that's longer than CFBundleName."
Comment 6 youenn fablet 2020-04-24 04:56:07 PDT
Created attachment 397446 [details]
Patch
Comment 7 youenn fablet 2020-04-24 05:43:31 PDT
Created attachment 397447 [details]
Patch
Comment 8 youenn fablet 2020-04-24 05:45:43 PDT
Created attachment 397448 [details]
Patch
Comment 9 youenn fablet 2020-04-24 06:55:16 PDT
Created attachment 397451 [details]
Patch
Comment 10 Geoffrey Garen 2020-04-24 09:31:14 PDT
Comment on attachment 397451 [details]
Patch

r=me
Comment 11 EWS 2020-04-24 10:02:21 PDT
Committed r260650: <https://trac.webkit.org/changeset/260650>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397451 [details].
Comment 12 Per Arne Vollan 2020-04-24 10:57:18 PDT
Comment on attachment 397451 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397451&action=review

> Source/WebKit/Scripts/process-entitlements.sh:155
> +    plistbuddy Add :com.apple.security.exception.mach-lookup.global-name array
> +    plistbuddy Add :com.apple.security.exception.mach-lookup.global-name:0 string com.apple.systemstatus.activityattribution

Is this also needed when issuing the extension?
Comment 13 Geoffrey Garen 2020-04-27 10:05:41 PDT
(In reply to Per Arne Vollan from comment #12)
> Comment on attachment 397451 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397451&action=review
> 
> > Source/WebKit/Scripts/process-entitlements.sh:155
> > +    plistbuddy Add :com.apple.security.exception.mach-lookup.global-name array
> > +    plistbuddy Add :com.apple.security.exception.mach-lookup.global-name:0 string com.apple.systemstatus.activityattribution
> 
> Is this also needed when issuing the extension?

Oh yeah, this looks like a mistake. The purpose of the extension is to avoid generally enabling access to this service in the WebContent process.
Comment 14 WebKit Commit Bot 2020-04-29 02:04:56 PDT
Re-opened since this is blocked by bug 211172
Comment 15 youenn fablet 2020-04-29 02:19:24 PDT
Created attachment 397946 [details]
Patch
Comment 16 EWS 2020-04-29 10:20:30 PDT
Committed r260904: <https://trac.webkit.org/changeset/260904>

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