1117771 - Consolidate checks for the iframe package in fireplace

Status: RESOLVED FIXED

(Marketplace Graveyard :: Code Quality, defect, P4)

Description

[Mathieu Pillard [:mat]]

There are several places in fireplace and marketplace-core-modules that check if we are in an iframed app, using window.self !== window.top. We should expose this in capabilities.js or some other module instead of repeating the same check everywhere.

Spotted by cvan when reviewing https://github.com/mozilla/fireplace/pull/877

Comment 1

[Mathieu Pillard [:mat]]

marketplace-core-modules core/capabilities module now exposes "packaged" and "iframed". We should find and replace the manual checks by these properties everywhere in fireplace (it's already used in fireplace/src/media/js/update_banner.js)

Comment 2

[Ram Dayal Vaishnav [:ram]]

Hi Kevin/MP-Team, it looks like this might not be very much clear to the newbie how to fix this. Can you put some information here for newbies so that they can pick this bug and fix it.

Thanks,
Ram

Comment 3

[Kevin Ngo [:ngoke]]

replace instances of window.self/window.top checks with required('capabilities').iframed

Comment 4

[Kevin Ngo [:ngoke]]

I mean require('core/capabilities')

Comment 5

[Mukul Jain]

I would like to take this bug

Comment 6

[Mukul Jain]

I would like to take this bug

Comment 7

[Ram Dayal Vaishnav [:ram]]

Thanks Mukul, Assigning this bug to you.

Comment 8

[Mukul Jain]

Sorry for double same comment(spam?!) it's by mistake, unable to remove!!!

i found "window.self !== window.top", only in one file which is webactivities.js, so do I have to replace that with "capabilities.iframed" ... that's what I have tried... I made a commit and pull request.

Comment 9

[Mathieu Pillard [:mat]]

I can't see your pull request at the moment (are you sure you submitted it ?) but that sounds right, there used to be one or two other files but they got cleaned up during other modifications.

Comment 10

[Mukul Jain]

Sorry I will make it again.

Comment 11

[Mukul Jain]

new pull request link : https://github.com/mozilla/fireplace/pull/1417

Comment 12

[Trishul]

Merged

Comment 13

[Ram Dayal Vaishnav [:ram]]

Many congratulations Mukul on fixing your good first bug :) As next steps

- Feel free to pick more good first bugs and work on them - https://bugzilla.mozilla.org/buglist.cgi?cmdtype=runnamed&namedcmd=mp-good-first-bugs
- We also encourage you to add your contribution to this month's recognition wiki - https://wiki.mozilla.org/Marketplace/Contributing/Aug2015#Contributor_Recognition