1173909 - ui-showcase injects CSS from both standalone and desktop into each iframe

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Right now, the ui-showcase injects the head from the containing document (which includes the stylesheets from both the desktop and standalone clients).  As a result, sometimes the rules conflict and make the showcase display something as it never gets displayed in reality.

We should be able to set up two different lists of CSS headers.  One of these list would be passed into each FramedExample.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Teach showcase and Frame about cssProps specific to iframes

Still needs a bit of documentation.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Note that this only fixes the problem in FramedExamples.  

In some other bug, we'll likely want to move all the Examples to FramedExamples so that this problem goes away completely.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Teach showcase and Frame about cssProps specific to iframes

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

I realize now that the patch here needs to be applied on top of the (not yet cleaned up) patch in bug 1173909.  I can either extract this one to be truly independent and land it first, or land them both together, once I've cleaned up the other patch. 

That said, this patch should be reviewable as-is, I think.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

I realize now that the patch here needs to be applied on top of the (not yet cleaned up) patch in bug 1168848.  I can either extract this one to be truly independent and land it first, or land them both together, once I've cleaned up the other patch. 

That said, this patch should be reviewable as-is, I think.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Teach showcase and Frame about cssProps specific to iframes

Rebased against recent fx-team.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Give loop showcase FramedExamples dashed borders and CSS style exclusion

This should apply cleanly to the fx-team branch, and fixes stuff I missed
the first time around.  It appears to be working as I would expect (it adds
dashed lines around many FramedExamples and the stylesheet isolation seems to be ok, however, there's a problem with the standalone footer not being styled
that I still need to debug, so I haven't yet requested review.

Comment 8

[Mark Banner (:standard8)]

Attachment: Additional fixes for bug 1173909

I added some debugging and found out what was happening. Although you'd added cssClass to window.Frame, the props were being added on FramedExample, but FramedExample wasn't tying the two together. So I fixed that, and then also realised that we need to explicitly check this.props.cssClass is defined before trying to do comparisons against it.

I've also removed a couple of .bind statements - react was posting lots of warnings to the console about the fact it does it in a more optimised way.

These two patches combined seem to work fine now.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8623382 [details] [diff] [review]
Give loop showcase FramedExamples dashed borders and CSS style exclusion

Review of attachment 8623382 [details] [diff] [review]:
-----------------------------------------------------------------

r=Standard8 as long as you land this with my other patch. I'm quite happy for you to combine them into one.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Give loop showcase FramedExamples dashed borders and CSS style exclusion

Combined patch for landing; carrying forward r=Standard8

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Landed on fx-team:

https://hg.mozilla.org/integration/fx-team/rev/d42c1dc7fbc1

Comment 12

[Carsten Book [:Tomcat]]

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