LibWeb: Pass a null sourceDocument to Browser UI initiated navigations #4515
Conversation
Upcoming HTML spec changes require us to create these without a document
Upcoming HTML changes will actually pass parameters requiring an empty TaskDestination to fetch from navigate(), so we need to be able to queue fetch tasks using the implicit parallel queue TaskDestination. Of course, we still have a FIXME for the parallel part, and just queue a task with the implicit document now instead..
| @@ -22,7 +26,7 @@ void upgrade_a_mixed_content_request_to_a_potentially_trustworthy_url_if_appropr | |||
| // 2. request’s URL’s host is an IP address. | |||
| || (request.url().host().has_value() && (request.url().host()->has<URL::IPv4Address>() || request.url().host()->has<URL::IPv6Address>())) | |||
|
|
|||
| // 3. § 4.3 Does settings prohibit mixed security contexts? returns "Does Not Restrict Mixed Security Contents" when applied to request’s client. | |||
| // 3. § 4.3 Does settings prohibit mixed security contexts? returns "Does Not Restrict Mixed Security Contents" when applied to request’s client. | |||
There was a problem hiding this comment.
Apparently this spec has some non-breaking spaces in it when you copy from it, caught my eye in the IDE.
| @@ -11,6 +11,15 @@ | |||
|
|
|||
| println(`transition is null: ${n.transition == null}`); | |||
| println(`canGoBack: ${n.canGoBack}`); | |||
There was a problem hiding this comment.
I have no idea why our test runner causes this to become false due to this change, but I can't say I think it should have been true to begin with.
| @@ -816,7 +816,23 @@ static WebIDL::ExceptionOr<Navigable::NavigationParamsVariant> create_navigation | |||
| request->set_referrer(entry->document_state()->request_referrer()); | |||
| request->set_policy_container(source_snapshot_params.source_policy_container); | |||
|
|
|||
| // 4. If documentResource is a POST resource, then: | |||
| // 4. If request's client is null, then: | |||
There was a problem hiding this comment.
Extra space here, and a couple more below you can find by searching for .
There was a problem hiding this comment.
artifact from copying steps from the whatpr url it seems. I'll do a scan.
| auto& active_document = *this->active_document(); | ||
| auto& realm = active_document.realm(); | ||
| auto& page_client = active_document.page().client(); | ||
|
|
||
| // AD-HOC: If we are not able to continue in this process, request a new process from the UI. | ||
| if (is_top_level_traversable() && !page_client.is_url_suitable_for_same_process_navigation(active_document.url(), params.url)) { | ||
| page_client.request_new_process_for_navigation(params.url); | ||
| if (!page_client.is_url_suitable_for_same_process_navigation(active_document.url(), url)) { |
There was a problem hiding this comment.
Merge conflict? We don't want to lose the is_top_level_traversable check. See 00aa620.
There was a problem hiding this comment.
oops yep, I had to do some gymnastics to rebase this, given your change and Shannon's change to make two of the types affected no longer take a realm when allocating
There was a problem hiding this comment.
Sorry, that came up when I was looking at that static function on Document function as well 😅 I also have this commit sitting around if helpful by the way: shannonbooth@c09aa7a
There was a problem hiding this comment.
At yeah that's the fetch spec algo that Domenic was updating due to my finding about the "request's window" needing updated for this.
There was a problem hiding this comment.
Right just a catchup commit to make the actual spec changes easier to see/apply in that fetch MR
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
|
This pull request has been closed because it has not had recent activity. Feel free to open a new pull request if you wish to still contribute these changes. Thank you for your contributions! |
Buildup and implementation for whatwg/html#11250
This does some .. interesting things to the m_current_entry_index of
window.navigation. But as far as I could tell, it doesn't actually regress our UI-initiated fwd/back buttons, so I guess we'll have to untangle that later :)