Bug 228624 - Document's fallback base URL should be deduced from its creator when URL is about:blank
Summary: Document's fallback base URL should be deduced from its creator when URL is a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://html.spec.whatwg.org/multipag...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-29 17:04 PDT by Chris Dumez
Modified: 2021-07-30 15:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.08 KB, patch)
2021-07-29 17:07 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (25.95 KB, patch)
2021-07-30 08:44 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.67 KB, patch)
2021-07-30 11:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-07-29 17:04:45 PDT
Document's fallback base URL should be deduced from its creator when URL is about:blank:
- https://html.spec.whatwg.org/multipage/urls-and-fetching.html#fallback-base-url

Chrome and Firefox match the specification here.
Comment 1 Chris Dumez 2021-07-29 17:07:42 PDT
Created attachment 434592 [details]
Patch
Comment 2 Chris Dumez 2021-07-30 08:44:08 PDT
Created attachment 434634 [details]
Patch
Comment 3 Chris Dumez 2021-07-30 11:23:44 PDT
Created attachment 434648 [details]
Patch
Comment 4 Geoffrey Garen 2021-07-30 13:00:12 PDT
Comment on attachment 434648 [details]
Patch

r=me
Comment 5 EWS 2021-07-30 13:25:53 PDT
Committed r280491 (240123@main): <https://commits.webkit.org/240123@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 434648 [details].
Comment 6 Radar WebKit Bug Importer 2021-07-30 13:26:18 PDT
<rdar://problem/81340285>
Comment 7 Darin Adler 2021-07-30 14:29:06 PDT
Comment on attachment 434648 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:3422
> +        if (m_baseURL == aboutBlankURL()) {

This is a case-sensitive check. Is that really what we want?
Comment 8 Chris Dumez 2021-07-30 14:53:54 PDT
Comment on attachment 434648 [details]
Patch

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

>> Source/WebCore/dom/Document.cpp:3422
>> +        if (m_baseURL == aboutBlankURL()) {
> 
> This is a case-sensitive check. Is that really what we want?

I can replace by m_baseURL.isAboutBlank() for clarity but it is also implemented like so:
```
bool URL::isAboutBlank() const
{
    return protocolIsAbout() && path() == "blank";
}
```

Note that the path() string check is case-sensitive. My understanding is that the path ("blank") is case-sensitive.

Note that here, I have just parsed m_baseURL, the URL parser lowercases the scheme/protocol so the protocol would be lower case no matter what here. Therefore, I think my check is identical to URL::isAboutBlank(). That said, I will switch to isAboutBlank() for clarity.
Comment 9 Chris Dumez 2021-07-30 15:04:06 PDT
(In reply to Chris Dumez from comment #8)
> Comment on attachment 434648 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=434648&action=review
> 
> >> Source/WebCore/dom/Document.cpp:3422
> >> +        if (m_baseURL == aboutBlankURL()) {
> > 
> > This is a case-sensitive check. Is that really what we want?
> 
> I can replace by m_baseURL.isAboutBlank() for clarity but it is also
> implemented like so:
> ```
> bool URL::isAboutBlank() const
> {
>     return protocolIsAbout() && path() == "blank";
> }
> ```
> 
> Note that the path() string check is case-sensitive. My understanding is
> that the path ("blank") is case-sensitive.
> 
> Note that here, I have just parsed m_baseURL, the URL parser lowercases the
> scheme/protocol so the protocol would be lower case no matter what here.
> Therefore, I think my check is identical to URL::isAboutBlank(). That said,
> I will switch to isAboutBlank() for clarity.

Switched to using URL::isAboutBlank() in <https://commits.webkit.org/r280498>.