Bug 222867 - [Flatpak SDK] Direct AVIF loading does not work.
Summary: [Flatpak SDK] Direct AVIF loading does not work.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-06 12:17 PST by ChangSeok Oh
Modified: 2021-05-04 22:26 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.33 KB, patch)
2021-03-07 09:36 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (5.47 KB, patch)
2021-04-25 08:48 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ChangSeok Oh 2021-03-06 12:17:39 PST
SSIA.
Comment 1 ChangSeok Oh 2021-03-06 17:03:44 PST
I investigated this issue in depth, and found that the root cause was flatpak sdk did not support avif mime type.

In webkit-side, we get a content type via g_file_info_get_content_type(info); in NetworkDataTaskSoup::didGetFileInfo. The returned type is 'application/octet-stream'. Why the glib returns the unknown content type is that xdg of flatpak sdk is not new enough to support avif. Please see https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/95

Actually I found two freedesktop.org.xml in my local webkit repo:
./WebKitBuild/UserFlatpak/runtime/org.webkit.Platform/x86_64/0.3/37c82c52d147824756b7cd2562549ad8cfcbc1ddf93e2aba33614a2325a70a07/files/share/mime/packages/freedesktop.org.xml
./WebKitBuild/UserFlatpak/runtime/org.webkit.Sdk/x86_64/0.3/e55e0606d22aee5dd1faeea8f35633363cae3e7a028efc0694631dc73b3d30b9/files/share/mime/packages/freedesktop.org.xml

None of them define the avif mime-type. Thus, updating xdg of flatpak SDK would fix this issue. @philipn, thought?
Comment 2 Philippe Normand 2021-03-07 09:35:01 PST
Sure, updating shared-mime-info to version 2.1 in the SDK fixes this bug :) And introduces thousands of layout test failures.

So somebody will need to bisect the regression in shared-mime-info. 1.10 good, 2.1 bad.
Comment 3 Philippe Normand 2021-03-07 09:36:43 PST
Created attachment 422531 [details]
Patch
Comment 4 ChangSeok Oh 2021-04-24 16:10:49 PDT
(In reply to Philippe Normand from comment #2)
> Sure, updating shared-mime-info to version 2.1 in the SDK fixes this bug :)
> And introduces thousands of layout test failures.
> 
> So somebody will need to bisect the regression in shared-mime-info. 1.10
> good, 2.1 bad.

Can we do that in local without bumping up the SDK on server? I mean, can I just update only my local SDK for bisecting?
Comment 5 Philippe Normand 2021-04-25 06:54:57 PDT
Yes, local SDK builds are possible:

webkit-flatpak-sdk --build all

Then install flatpak image locally:

webkit-flatpak --repo=$PWD/Tools/buildstream/repo -u

Then set this env var when running build-webkit/etc:

WEBKIT_FLATPAK_USER_DIR=$PWD/WebKitBuild/UserFlatpak.Local


The issue though is that this regression is quite old, see also bug 202321... gio detects this file LayoutTests/fast/xsl/extra-lf-at-end.html as xhtml for instance when the shared-mime-info db was built with shared-mime-info 2.1, whereas with a db built with 1.10, text/html is detected.

I'm not sure what to do... it's not the first time we have issues with shared-mime-info... I wonder if we should use that only as last resort and use the MIMETypeRegistry as first option.
Comment 6 Philippe Normand 2021-04-25 08:48:19 PDT
Created attachment 427002 [details]
Patch
Comment 7 Michael Catanzaro 2021-04-26 07:27:04 PDT
(In reply to Philippe Normand from comment #5)
> I'm not sure what to do... it's not the first time we have issues with
> shared-mime-info... I wonder if we should use that only as last resort and
> use the MIMETypeRegistry as first option.

I don't know, maybe. Our MIME type detection seems to be generally quite fragile and incompatible with what other browsers are doing....
Comment 8 Michael Catanzaro 2021-04-26 14:33:51 PDT
Comment on attachment 427002 [details]
Patch

Please get Carlos Garcia's opinion before landing, but I think it makes sense. We have too many problems with misdetected MIME types and I wouldn't be surprised if this fixes other bugs too.

We might even consider removing extractMIMETypeFromMediaType altogether if that's what would best match the behavior of other browsers.
Comment 9 Jon Bauman 2021-04-26 15:56:50 PDT
(In reply to Michael Catanzaro from comment #8)
> We might even consider removing extractMIMETypeFromMediaType altogether if
> that's what would best match the behavior of other browsers.

Regarding Firefox, https://bugzilla.mozilla.org/show_bug.cgi?id=1656099 may be of interest
Comment 10 Michael Catanzaro 2021-04-26 16:09:21 PDT
(In reply to Jon Bauman from comment #9)
> Regarding Firefox, https://bugzilla.mozilla.org/show_bug.cgi?id=1656099 may
> be of interest

Interestingly, that looks like the opposite of what we're doing here, as this patch causes the file content to no longer be used.
Comment 11 Carlos Garcia Campos 2021-04-27 00:14:23 PDT
Comment on attachment 427002 [details]
Patch

LGTM
Comment 12 Philippe Normand 2021-04-27 00:35:07 PDT
(In reply to Michael Catanzaro from comment #10)
> (In reply to Jon Bauman from comment #9)
> > Regarding Firefox, https://bugzilla.mozilla.org/show_bug.cgi?id=1656099 may
> > be of interest
> 
> Interestingly, that looks like the opposite of what we're doing here, as
> this patch causes the file content to no longer be used.

For local files yes.
For remote assets the contents is still sniffed, afaik.
Comment 13 EWS 2021-04-27 04:50:53 PDT
Committed r276635 (237063@main): <https://commits.webkit.org/237063@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427002 [details].
Comment 14 Radar WebKit Bug Importer 2021-04-27 04:51:14 PDT
<rdar://problem/77204106>