Bug 236069 – REGRESSION (iOS 15.1 / r280824) QuickLook - model not loading when passing extra parameters

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Maciek Główka]

On iOS 15.2 when passing USDZ from Safari to QuickLook AR, as a blob with parameters (eg. fixed scaling), the model is never rendered.
The loader spinner keeps spinning infinitely.

You can repro the bug here:
http://tests.glowka.atthost24.pl/

Built using model-viewer.dev component. USDZ is generated on the fly.
I believe model-viewer's 'fixed scaling' is translated as #allowsContentScaling=0
Without the parameter models loads as expected.

Possibly related to:
https://bugs.webkit.org/show_bug.cgi?id=226265

[Radar WebKit Bug Importer]

<rdar://problem/88461772>

[Antoine Quint]

We get this error in [_WKPreviewControllerDataSource failWithError].

Domain=WebKitBlobResource Code=1 "(null)" UserInfo={NSErrorFailingURLKey=blob:http://tests.glowka.atthost24.pl/24701d34-f863-4935-af4e-8b13c1ee9492#allowsContentScaling=0, NSErrorFailingURLStringKey=blob:http://tests.glowka.atthost24.pl/24701d34-f863-4935-af4e-8b13c1ee9492#allowsContentScaling=0}

[Antoine Quint]

This regressed with r280824, the fix for bug 228923.

[Antoine Quint]

To whomever may work on this: this type of content leads to SystemPreviewControllerCocoa trying to open a blob URL for a failed download.

[Chris Dumez]

I know what's going on and I am validating a fix.

[Chris Dumez]

Created attachment 455385 [details]
WIP Patch (needs test)

[Chris Dumez]

Created attachment 455388 [details]
Patch

[Maciek Główka]

Thanks for taking care of the bug!

Will the patch push the fixed-scale parameter to QuickLook as well or is it only model-not-loading-at-all fix ?

Regards,
Maciek

[Chris Dumez]

(In reply to Maciek Główka from comment #8)
> Thanks for taking care of the bug!
> 
> Will the patch push the fixed-scale parameter to QuickLook as well or is it
> only model-not-loading-at-all fix ?
> 
> Regards,
> Maciek

I know very little about USDZ but I am keeping the fragment as part of the blob URL (just making the blob download actually succeed if its URL contains a fragment) so I would expect it to be passed to QuickLook.

If you tell me exactly how to validate though, I am happy to do so.

[Maciek Główka]

Thanks. I guess if the #allowsContentScaling=0 is passed it should be sufficient.
As far as I know when .usdz file is given with the fragment it's working as expected. Only the blobs were problematic.

[Darin Adler]

Comment on attachment 455388 [details]
Patch

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

> Source/WebCore/platform/network/BlobRegistryImpl.cpp:56
> +static String blobURLWithoutFragment(const URL& url)
> +{
> +    return url.hasFragmentIdentifier() ? url.stringWithoutFragmentIdentifier().toString() : url.string();
> +}

This should probably eventually be a URL member function rather than a blob-specific helper, even if not in this patch. Generally things that return StringView but we want to optimize the "unchanged pre-existing String" case are a recurring pattern. Just need the right name, I suppose. Maybe we need to rename the existing function to viewWithoutFragmentIdentifier and this operation can be stringWithoutFragmentIdentifier, but I hate to have this here.

[Chris Dumez]

Comment on attachment 455388 [details]
Patch

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

>> Source/WebCore/platform/network/BlobRegistryImpl.cpp:56
>> +}
> 
> This should probably eventually be a URL member function rather than a blob-specific helper, even if not in this patch. Generally things that return StringView but we want to optimize the "unchanged pre-existing String" case are a recurring pattern. Just need the right name, I suppose. Maybe we need to rename the existing function to viewWithoutFragmentIdentifier and this operation can be stringWithoutFragmentIdentifier, but I hate to have this here.

I like the idea. I'll implement this in a follow-up though to facilitate cherry-picking of this patch to branches.

[EWS]

Committed r291689 (248731@main): <https://commits.webkit.org/248731@main>

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

[Miles]

This issue still exists in iOS 15.5 beta (19F5047e)

[Antoine Quint]

(In reply to Miles from comment #14)
> This issue still exists in iOS 15.5 beta (19F5047e)

Yes, the code change is not in that beta release. I can't comment on when it will be available just yet, but that is expected.

[Miles]

Thanks for the update. 
I'll keep watching in anticipation. 
Cheers

[Miles]

Just FYI... This issue still exists in iOS 15.5 beta 2 (19F5057e)
I'll retest with the next update. 
Thanks.

[Miles]

Issue still exists in iOS 15.5 beta 3 (19F5062g)
I live in hope for the next update ;)

[Miles]

This looks to be resolved in iOS 15.5 beta 4 (19F5070b).
So far I have tested opening USDZ as blob in AR QuickLook along with #allowsContentScaling=0 parameter, and the model does now load and QuickLook does indeed correctly prevent user from scaling the 3D model. Great!

[Brent Fulgham]

This fix shipped with Safari 15.5 (all platforms).