See https://github.com/w3c/manifest/issues/550#issuecomment-361129547 The spec is likely going to be updated to match Chrome's behavior, which some web apps rely on.
<rdar://problem/37093498>
Created attachment 332910 [details] Patch
Comment on attachment 332910 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332910&action=review > Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:211 > + URL result { url }; > + > + result.removeQueryAndFragmentIdentifier(); > + > + auto path = result.path(); > + if (path.endsWith('/')) > + return result; > + > + auto lastSlash = path.reverseFind('/'); > + if (lastSlash == WTF::notFound) { > + result.setPath({ }); > + return result; > + } > + > + result.setPath(path.substring(0, lastSlash + 1)); > + return result; Just do: URL(url, "./"). What's what service worker code does. Also, we should this help function scopeURL or computeScopeURL or something. "containing URL" is not really a nominal terminology.
(In reply to Ryosuke Niwa from comment #3) > Comment on attachment 332910 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332910&action=review > > > Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:211 > > + URL result { url }; > > + > > + result.removeQueryAndFragmentIdentifier(); > > + > > + auto path = result.path(); > > + if (path.endsWith('/')) > > + return result; > > + > > + auto lastSlash = path.reverseFind('/'); > > + if (lastSlash == WTF::notFound) { > > + result.setPath({ }); > > + return result; > > + } > > + > > + result.setPath(path.substring(0, lastSlash + 1)); > > + return result; > > Just do: URL(url, "./"). What's what service worker code does. That's a lot cleaner. I'll use that instead. > Also, we should this help function scopeURL or computeScopeURL or something. > "containing URL" is not really a nominal terminology.
Created attachment 332973 [details] Patch v2
Comment on attachment 332973 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=332973&action=review > Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:198 > - return { }; > + return { startURL, "./" }; Hm... I would have preferred to have a helper function like you did earlier.
(In reply to Ryosuke Niwa from comment #6) > Comment on attachment 332973 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332973&action=review > > > Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:198 > > - return { }; > > + return { startURL, "./" }; > > Hm... I would have preferred to have a helper function like you did earlier. I'm open to adding back a helper function. How does `void defaultScopeURL(const URL& startURL)` sound?
Comment on attachment 332973 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=332973&action=review >>> Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:198 >>> + return { startURL, "./" }; >> >> Hm... I would have preferred to have a helper function like you did earlier. > > I'm open to adding back a helper function. How does `void defaultScopeURL(const URL& startURL)` sound? Sure that sounds good. Alternatively, you could just compute it once at the top of the function as defaultScopeURL local variable since this isn't a perf sensitive code.
Created attachment 333011 [details] Patch for landing
Comment on attachment 333011 [details] Patch for landing Clearing flags on attachment: 333011 Committed r228036: <https://trac.webkit.org/changeset/228036>
All reviewed patches have been landed. Closing bug.