Bug 228965

Summary: Meta HTTP refresh should not navigate if document has sandboxed automatic features browsing context flag set
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, changseok, darin, esprehn+autocc, ews-watchlist, ggaren, gyuyoung.kim, hi, japhet, kangil.han, mkwst, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2021-08-10 12:21:56 PDT
Meta HTTP refresh should not navigate if document has sandboxed automatic features browsing context flag set:
- https://html.spec.whatwg.org/multipage/semantics.html#shared-declarative-refresh-steps (Step 13)

Firefox and Chrome already behave this way.
Comment 1 Chris Dumez 2021-08-10 12:26:39 PDT
Created attachment 435281 [details]
Patch
Comment 2 Darin Adler 2021-08-10 15:05:20 PDT
Comment on attachment 435281 [details]
Patch

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

> Source/WebCore/loader/NavigationScheduler.cpp:186
> +            if (auto* document = frame.document()) {
> +                if (document->isSandboxed(SandboxAutomaticFeatures)) {

Rather than nesting, I like this style:

    if (auto document = frame.document(); document && document->isSandboxed(SandboxAutomaticFeatures) {
Comment 3 Darin Adler 2021-08-10 15:05:45 PDT
Comment on attachment 435281 [details]
Patch

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

>> Source/WebCore/loader/NavigationScheduler.cpp:186
>> +                if (document->isSandboxed(SandboxAutomaticFeatures)) {
> 
> Rather than nesting, I like this style:
> 
>     if (auto document = frame.document(); document && document->isSandboxed(SandboxAutomaticFeatures) {

Not saying you must change this, just telling you the style I prefer.
Comment 4 Chris Dumez 2021-08-10 15:06:41 PDT
(In reply to Darin Adler from comment #3)
> Comment on attachment 435281 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=435281&action=review
> 
> >> Source/WebCore/loader/NavigationScheduler.cpp:186
> >> +                if (document->isSandboxed(SandboxAutomaticFeatures)) {
> > 
> > Rather than nesting, I like this style:
> > 
> >     if (auto document = frame.document(); document && document->isSandboxed(SandboxAutomaticFeatures) {
> 
> Not saying you must change this, just telling you the style I prefer.

Oh, indeed. It is nicer and will make the update. My brain just hasn't adjusted to the new C++ syntax yet :)
Comment 5 Chris Dumez 2021-08-10 15:14:34 PDT
Created attachment 435302 [details]
Patch
Comment 6 EWS 2021-08-10 16:19:11 PDT
Committed r280870 (240409@main): <https://commits.webkit.org/240409@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435302 [details].
Comment 7 Radar WebKit Bug Importer 2021-08-10 16:20:23 PDT
<rdar://problem/81766558>