Bug 100735 – X-Frame-Options console message should be associated with a request.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

Currently, a frame blocked via X-Frame-Options in a meta tag sends "Refused to display document because display forbidden by X-Frame-Options." We should associate this message with the request that loaded the document in order to pull a stack trace/line number.

[Mike West]

Created attachment 171407 [details]
Patch

[Mike West]

Hi Pavel, Adam.

This patch depends on #99941, which adds the ability to tie a console message to a request. I'll throw it to the bots when that patch lands; it'll explode otherwise.

I'm not entirely sure I'm grabbing the request identifier correctly inside of Document. Adam, it looks like you added the console message there: can you evaluate the approach I've taken, as well as the new error message?

Pavel, this is more FYI for you. If you'd like me to move the additions to ScriptExecutionContext::addConsoleMessage into this patch and out of 99941, I'm happy to.

Thanks!

[Mike West]

Actually CCing Adam and Pavel. See https://bugs.webkit.org/show_bug.cgi?id=100735#c2 for what should have been attached to this email. :)

[Mike West]

Hrm. It went to the bots anyway. Oops.

[Build Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14631461

[WebKit Review Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14632434

[Early Warning System Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14631463

[Early Warning System Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14572059

[Peter Beverloo (cr-android ews)]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14632435

[EFL EWS Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14627488

[Build Bot]

Comment on attachment 171407 [details]
Patch

Attachment 171407 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14629470

[Pavel Feldman]

Comment on attachment 171407 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:2962
> +                String message = "Refused to display '" + url().string() + "' in a frame because it set 'X-Frame-Options' to '" + content + "'.";

Do you intend to localize these?

[Mike West]

(In reply to comment #12)
> (From update of attachment 171407 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171407&action=review
> 
> > Source/WebCore/dom/Document.cpp:2962
> > +                String message = "Refused to display '" + url().string() + "' in a frame because it set 'X-Frame-Options' to '" + content + "'.";
> 
> Do you intend to localize these?

Hrm. Hadn't thought about it, honestly. :)

Would that just involve adding another function to LocalizedString.cpp/h?

[Pavel Feldman]

> Would that just involve adding another function to LocalizedString.cpp/h?

We don't really have a policy for that. It sounds like DOM exceptions are not localized, so it is up to you. I can only see context menus in LocalizedStrings.cpp + Chromium does not use those at all.

[Mike West]

(In reply to comment #14)
> > Would that just involve adding another function to LocalizedString.cpp/h?
> 
> We don't really have a policy for that. It sounds like DOM exceptions are not localized, so it is up to you. I can only see context menus in LocalizedStrings.cpp + Chromium does not use those at all.

I've looked through the code; we don't localize any exceptions at all (at the moment (that I found)).

I think it might be worth doing, but not in this patch. :)

[Mike West]

Created attachment 171603 [details]
Patch for landing.

[WebKit Review Bot]

Comment on attachment 171603 [details]
Patch for landing.

Clearing flags on attachment: 171603

Committed r133019: <http://trac.webkit.org/changeset/133019>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.