1445106 - Prevent feed reader from being framed

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, enhancement, P3)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

We show a custom page (about:feeds) when encountering RSS feed, so the user is presented with something nicer than raw XML. While this looks like web content, it isn't and it probably should be able to be framed, since there is some browser UI there. While the UI in the page isn't security sensitive (all decisions are handled in the parent, usually via pop-up subscription confirmations etc) websites probably shouldn't' frame these pages as this might confuse users. Should be simple fix to prevent framing, so filing this bug to enforce this.

Comment 1

[Marco Bonardo [:mak]]

A discussion is ongoing regarding the future of this component, I'm setting P3 because it's something that ideally should be fixed by the frontend team, but hopefully it will become a non-issue soon.

Comment 2

[:Gijs (he/him)]

(In reply to Marco Bonardo [::mak] from comment #1)
> A discussion is ongoing regarding the future of this component, I'm setting
> P3 because it's something that ideally should be fixed by the frontend team,
> but hopefully it will become a non-issue soon.

FWIW, I think we should probably still do this for 60, because the removal definitely won't happen for then.

I'll take this.

Comment 3

[Marco Bonardo [:mak]]

Sounds good, I'm available to review your patch.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1445106 - don't try to have a feed reader in a frame,

Review commit: https://reviewboard.mozilla.org/r/227792/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/227792/

Comment 5

[:Gijs (he/him)]

Doing it in 3 different places is probably a bit over the top, but better safe than sorry...

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 8958929 [details]
Bug 1445106 - don't try to have a feed reader in a frame,

https://reviewboard.mozilla.org/r/227792/#review233824

I assume you tested the expected basic functionality works properly

Comment 7

[:Gijs (he/him)]

Comment on attachment 8958929 [details]
Bug 1445106 - don't try to have a feed reader in a frame,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227792/diff/1-2/

Comment 8

[:Gijs (he/him)]

Realized that a bunch of tests break now because they frame the rss reader (because that's what mochitest do with everything). Added a pref to fix.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e7c237081dd34d216e28c4d8bf5f70fb8b8f546

Hopefully this is green. Marco, if you want to have another look you can. :-)

(I didn't change the feed sniffing thing to read a pref... in my local testing that wasn't necessary, but let's see what automation says...)

Comment 9

[Marco Bonardo [:mak]]

The try still looks broken?

Comment 10

[:Gijs (he/him)]

(In reply to Marco Bonardo [::mak] from comment #9)
> The try still looks broken?

Ugh, I think I forgot to rebuild the native part, which is why I thought this should pass. I guess that needs a pref check, too. :-(

Comment 11

[:Gijs (he/him)]

Comment on attachment 8958929 [details]
Bug 1445106 - don't try to have a feed reader in a frame,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227792/diff/2-3/

Comment 12

[:Gijs (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc1cca9da0afec48daef3b343c1075c14ae863b6&selectedJob=169296266

looks better. Marco, can you doublecheck the changes look OK to you?

Comment 13

[Marco Bonardo [:mak]]

it looks ok, and thank you for moving around the tests.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a10873c09672
don't try to have a feed reader in a frame, r=mak

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/a10873c09672