-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Spec] Fix navigation from the default constructor not installing a fenced frame config. #183
Conversation
I assumed the changes described here would match the implementation but that doesn't seem to be the case in https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/fenced_frame/html_fenced_frame_element.cc;l=518-535;drc=82dff63dbf9db05e9274e11d9128af7b9f51ceaa, or the constructor which doesn't store or create an underlying config in default mode, right? |
There's no underlying config being added to the urn/config map, but the navigation request is creating a new fenced frame properties object that gets installed in the resulting navigated document. So we do need a I think the way to have this match the implementation is to have the navigation algorithm directly create a new |
Yep this is exactly what I was thinking too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM with small nits
The existing spec results in a fenced frame navigating directly to the
url
stored in aFencedFrameConfig
IDL object when navigating to a config built with theFencedFrameConfig(url)
default constructor. This results in afenced frame config
andfenced frame config instance
not being created for the fenced frame, cutting off the internal document fromwindow.fence
.This PR fixes that in the following ways:
FencedFrameConfig(url)
constructor to also create afenced frame config
and add it to the navigable'sfenced frame config mapping
.FencedFrameConfig
'surn
when navigating the internal document, ensuring that the navigation will find the associatedfenced frame config
, and instantiate afenced frame config instance
as expected.url
field from theFencedFrameConfig
in favor of using themapped url
in thefenced frame config
/fenced frame config instance
.FencedFrameConfig
directly in the finalized mapping without first putting it in the pending mapping.This PR also adds a
default fenced frame effective sandboxing flags
constant, which is needed when building thefenced frame config
in theFencedFrameConfing(url)
constructor.See issue: #178
Preview | Diff