1601887 - XFO: Display error page but still fire onload event in case x-frame-options blocks a load

Status: RESOLVED FIXED

(Core :: DOM: Security, task)

Description

[Christoph Kerschbaumer [:ckerschb]]

For web-compatibility reasons we decided not to display the newly added error page for x-frame-options added within Bug 1584998, but to go back and display about:blank because about:blank automagically fires the .onload() event which apparently some web pages rely on.

Within this bug we should:

• Make sure to display the new error page by using channel->Cancel(NS_ERROR_XFO_VIOLATION)
• make sure this new error page fires the onload() event handler
• verify that web-platform tests work x-frame-options work correctly.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Smaug, as agreed within https://bugzilla.mozilla.org/show_bug.cgi?id=1599256#c15 we were going back to load about:blank which fires the onload event in case x-frame-options blocks an iframe load. Within this Bug we would like to display the error page but still fire the onload event. Any pointers on how, and in particular where we would have to add code, to accomplish that?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

If we start to fire load event on error loads (I assume other browsers do that in all the cases?), I'd prefer if we could handle that
here https://searchfox.org/mozilla-central/rev/a92ed79b0bc746159fc31af1586adbfa9e45e264/layout/base/nsDocumentViewer.cpp#1022,1027
There we currently check explicitly success of aStatus.
But I'm not sure if we get there with all different kinds of error loads.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1601887 - XFO: Display error page but still fire onload event in case x-frame-options blocks a load.r=smaug

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Henri, did you ever try to figure out load event firing in error load cases in other browsers?
Or any other feedback to this bug?
(feel free to just clear the needinfo if you don't have, or if it is more like "I'd need to read the code" :) )

I'd still prefer some generic setup for load event firing for error cases.

Christoph, could you explain why many tests now fail, not timeout?
What is missing to get the tests pass?

Comment 5

[Henri Sivonen (:hsivonen)]

Sorry, I don't have any insights for this bug.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Olli Pettay [:smaug] from comment #4)

Christoph, could you explain why many tests now fail, not timeout?

Previously we loaded about:blank which caused the onload event to fire. Now we are loading an error page and tests are basically checking

testframe.contentDocument == null

which is not the case because loading the error page resuts in [object HTMLDocument].

What is missing to get the tests pass?

Otherwise tests would pass.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Huh, are we exposing the error page to the web? I don't think we should. Does it get somehow wrong principal.
The old code explicitly creates null principal: NullPrincipal::CreateWithInheritedAttributes

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/631d2c00da96
XFO: Display error page but still fire onload event in case x-frame-options blocks a load.r=smaug

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/631d2c00da96