[webkit-reviews] review granted: [Bug 173162] Align Document::canNavigate on the HTM5 specification : [Attachment 314011] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 173162: Align Document::canNavigate on the HTM5 specification
https://bugs.webkit.org/show_bug.cgi?id=173162

Attachment 314011: Patch

https://bugs.webkit.org/attachment.cgi?id=314011&action=review




--- Comment #39 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 314011
  --> https://bugs.webkit.org/attachment.cgi?id=314011
Patch

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

> Source/WebCore/dom/Document.cpp:-3103
> -    if (!isSandboxed(SandboxTopNavigation) && targetFrame ==
&m_frame->tree().top())

Since we need to keep this logic, let's keep it here instead of moving it
below.

> Source/WebCore/dom/Document.cpp:-3107
> -	   if (targetFrame->tree().isDescendantOf(m_frame))

Dropping this makes us stricter and I worry about making our behavior stricter
at this point. Can we please keep this? The spec says to return true in step 4
but we currently do not (we have some extra origin checks). Therefore, keeping
this check here keeps us closer to the spec as far as I can tell. So something
like:
if (isSandboxed(SandboxNavigation) &&
targetFrame->tree().isDescendantOf(m_frame))
    return true;

> Source/WebCore/dom/Document.cpp:3126
>      // This is the normal case. A document can navigate its decendant
frames,

Can you please fix the "decendant" typo while you're here.

>> Source/WebCore/dom/Document.cpp:3156
>> +	if (!isSandboxed(SandboxTopNavigation) && targetFrame ==
&m_frame->tree().top())
> 
> So we allow a subframe to navigate its top frame if it has
'allow-top-navigation', even if it is cross-origin? I suppose you kept this to
address layout test failures? Is this how other browsers behave?

Looks like this is fine but I'd rather we keep it where it was, at the
beginning of this method.