Bug 237089

Summary: Bubblewrap launcher doesn't bind font locations from XDG_DATA_DIRS
Product: WebKit Reporter: apteryx <maxim.cournoyer>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, cgarcia, mcatanzaro, pgriffis
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 206533    
Attachments:
Description Flags
Patch
none
Patch none

Description apteryx 2022-02-23 08:52:49 PST 
The BubbleWrap launcher of WebKitGTK doesn't expose font locations present in XDG_DATA_DIRS.  This problem was originally reported by a Guix user at https://issues.guix.gnu.org/41174.
Comment 1 apteryx 2022-02-23 13:03:21 PST 
Created attachment 453016 [details]
Patch
Comment 2 Patrick Griffis 2022-02-23 14:03:46 PST 
Looks reasonable.
Comment 3 Michael Catanzaro 2022-02-23 14:10:00 PST 
Comment on attachment 453016 [details]
Patch

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

I agree, except for code style nits:

> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:308
> +    for (auto dataDir = dataDirs; dataDir != nullptr && *dataDir != nullptr; dataDir++) {

I'm surprised the style bot didn't complain about this. Always write out the * on auto* to make it more clear that it's a pointer, and never explicitly compare against nullptr:

for (auto* dataDir = dataDirs; dataDir && *dataDir; dataDir++) {
Comment 4 Michael Catanzaro 2022-02-23 14:10:34 PST 
Comment on attachment 453016 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        Expose all fonts locations present in XDG_DATA_DIRS

First line should match the title of the bug:

Bubblewrap launcher doesn't bind font locations from XDG_DATA_DIRS
Comment 5 apteryx 2022-02-23 14:42:00 PST 
Created attachment 453031 [details]
Patch
Comment 6 apteryx 2022-02-23 14:42:53 PST 
(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 453016 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453016&action=review
> 
> I agree, except for code style nits:
> 
> > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:308
> > +    for (auto dataDir = dataDirs; dataDir != nullptr && *dataDir != nullptr; dataDir++) {
> 
> I'm surprised the style bot didn't complain about this. Always write out the
> * on auto* to make it more clear that it's a pointer, and never explicitly
> compare against nullptr:
> 
> for (auto* dataDir = dataDirs; dataDir && *dataDir; dataDir++) {

Adjusted. Thanks for the prompt review!
Comment 7 apteryx 2022-02-23 14:43:08 PST 
(In reply to Michael Catanzaro from comment #4)
> Comment on attachment 453016 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453016&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        Expose all fonts locations present in XDG_DATA_DIRS
> 
> First line should match the title of the bug:
> 
> Bubblewrap launcher doesn't bind font locations from XDG_DATA_DIRS

Done.
Comment 8 EWS 2022-02-23 15:48:39 PST 
Committed r290395 (247709@main): <https://commits.webkit.org/247709@main>

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