Bug 237089 - Bubblewrap launcher doesn't bind font locations from XDG_DATA_DIRS
Summary: Bubblewrap launcher doesn't bind font locations from XDG_DATA_DIRS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: Bubblewrap
  Show dependency tree / graph
 
Reported: 2022-02-23 08:52 PST by apteryx
Modified: 2022-02-23 15:48 PST (History)
5 users (show)

See Also:

Attachments
Patch (2.05 KB, patch)
2022-02-23 13:03 PST, apteryx 		no flags Details | Formatted Diff | Diff
Patch (2.05 KB, patch)
2022-02-23 14:42 PST, apteryx 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 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].